From 786f19bba7c4fac01daacceeca58f9bc2a2709cb Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Wed, 28 Jan 2026 15:11:55 +0100 Subject: [PATCH 01/11] Made param_t const --- include/param/param.h | 42 ++++++------ include/param/param_list.h | 16 ++--- include/param/param_queue.h | 2 +- include/param/param_serializer.h | 6 +- include/param/param_string.h | 4 +- src/param/collector/param_collector_config.h | 6 +- src/param/list/param_list.c | 68 ++++++++------------ src/param/param.c | 30 ++++----- src/param/param_client.c | 2 +- src/param/param_queue.c | 8 +-- src/param/param_serializer.c | 8 +-- src/param/param_server.c | 8 +-- src/param/param_string.c | 10 +-- src/vmem/vmem_server.c | 2 +- 14 files changed, 99 insertions(+), 113 deletions(-) diff --git a/include/param/param.h b/include/param/param.h index dfc872f2..63c33000 100644 --- a/include/param/param.h +++ b/include/param/param.h @@ -110,7 +110,7 @@ typedef struct param_s { int array_step; /* Local info */ - void (*callback)(struct param_s * param, int offset); + void (*callback)(const struct param_s * param, int offset); #ifdef PARAM_HAVE_TIMESTAMP csp_timestamp_t * timestamp; #endif @@ -155,8 +155,8 @@ typedef struct param_s { PARAM_TIMESTAMP_DECL(_name) \ uint16_t _node_##_name = 0; \ __attribute__((section("param"))) \ - __attribute__((used, no_reorder)) \ - param_t _name = { \ + __attribute__((used, no_reorder, aligned(8))) \ + const param_t _name = { \ .vmem = NULL, \ .node = &_node_##_name, \ .id = _id, \ @@ -178,8 +178,8 @@ typedef struct param_s { PARAM_TIMESTAMP_DECL(_name) \ uint16_t _node_##_name = 0; \ __attribute__((section("param"))) \ - __attribute__((used, no_reorder)) \ - param_t _name = { \ + __attribute__((used, no_reorder, aligned(8))) \ + const param_t _name = { \ .node = &_node_##_name, \ .id = _id, \ .type = _type, \ @@ -202,8 +202,8 @@ typedef struct param_s { ; /* Catch const param defines */ \ PARAM_TIMESTAMP_DECL(_name) \ __attribute__((section("param"))) \ - __attribute__((used, no_reorder)) \ - param_t _name = { \ + __attribute__((used, no_reorder, aligned(8))) \ + const param_t _name = { \ .node = _nodeaddr, \ .id = _id, \ .type = _type, \ @@ -239,8 +239,8 @@ typedef struct param_s { /* Native getter functions, will return native types */ #define PARAM_GET(type, name) \ - type param_get_##name(param_t * param); \ - type param_get_##name##_array(param_t * param, unsigned int i); + type param_get_##name(const param_t * param); \ + type param_get_##name##_array(const param_t * param, unsigned int i); PARAM_GET(uint8_t, uint8) PARAM_GET(uint16_t, uint16) PARAM_GET(uint32_t, uint32) @@ -255,10 +255,10 @@ PARAM_GET(double, double) /* Native setter functions, these take a native type as argument */ #define PARAM_SET(type, name) \ - void param_set_##name(param_t * param, type value); \ - void param_set_##name##_nocallback(param_t * param, type value); \ - void param_set_##name##_array(param_t * param, unsigned int i, type value); \ - void param_set_##name##_array_nocallback(param_t * param, unsigned int i, type value); + void param_set_##name(const param_t * param, type value); \ + void param_set_##name##_nocallback(const param_t * param, type value); \ + void param_set_##name##_array(const param_t * param, unsigned int i, type value); \ + void param_set_##name##_array_nocallback(const param_t * param, unsigned int i, type value); PARAM_SET(uint8_t, uint8) PARAM_SET(uint16_t, uint16) PARAM_SET(uint32_t, uint32) @@ -272,24 +272,24 @@ PARAM_SET(double, double) #undef PARAM_SET /* Non-native types needs to go through a function which includes a void pointer and the length */ -void param_set_data(param_t * param, const void * inbuf, int len); -void param_set_data_nocallback(param_t * param, const void * inbuf, int len); -void param_get_data(param_t * param, void * outbuf, int len); -void param_set_string(param_t * param, const char * inbuf, int len); +void param_set_data(const param_t * param, const void * inbuf, int len); +void param_set_data_nocallback(const param_t * param, const void * inbuf, int len); +void param_get_data(const param_t * param, void * outbuf, int len); +void param_set_string(const param_t * param, const char * inbuf, int len); #define param_get_string param_get_data /* Generic setter function: * This function can be used to set data of any type */ -void param_set(param_t * param, unsigned int offset, void * value); -void param_get(param_t * param, unsigned int offset, void * value); +void param_set(const param_t * param, unsigned int offset, void * value); +void param_get(const param_t * param, unsigned int offset, void * value); /* Returns the size of a native type */ int param_typesize(param_type_e type); -int param_size(param_t * param); +int param_size(const param_t * param); /* Copies from one parameter to another */ -void param_copy(param_t * dest, param_t * src); +void param_copy(const param_t * dest, const param_t * src); /* External hooks to get atomic writes */ extern __attribute__((weak)) void param_enter_critical(void); diff --git a/include/param/param_list.h b/include/param/param_list.h index e5144a37..35906d6f 100644 --- a/include/param/param_list.h +++ b/include/param/param_list.h @@ -17,10 +17,10 @@ extern "C" { typedef struct param_list_iterator_s { int phase; // Hybrid iterator has multiple phases (0 == Static, 1 == Dynamic List) - param_t * element; + const param_t * element; } param_list_iterator; -param_t * param_list_iterate(param_list_iterator * iterator); +const param_t * param_list_iterate(param_list_iterator * iterator); int param_list_add(param_t * item); @@ -40,13 +40,13 @@ int param_list_remove(int node, uint8_t verbose); * @param verbose Whether to print the removed parameter. * @return int 1 if the parameter was found and removed. */ -void param_list_remove_specific(param_t * param, uint8_t verbose, int destroy); -param_t * param_list_find_id(int node, int id); -param_t * param_list_find_name(int node, const char * name); +void param_list_remove_specific(const param_t * param, uint8_t verbose, int destroy); +const param_t * param_list_find_id(int node, int id); +const param_t * param_list_find_name(int node, const char * name); void param_list_print(uint32_t mask, int node, const char * globstr, int verbosity); uint32_t param_maskstr_to_mask(const char * str); -param_t * param_list_from_line(const char * line); +const param_t * param_list_from_line(const char * line); /** * @brief @@ -64,8 +64,8 @@ param_t * param_list_from_line(const char * line); */ param_t * param_list_create_remote(int id, int node, int type, uint32_t mask, int array_size, char * name, char * unit, char * help, int storage_type); -void param_list_destroy(param_t * param); -void param_print(param_t * param, int offset, int nodes[], int nodes_count, int verbose, uint32_t ref_timestamp); +void param_list_destroy(const param_t * param); +void param_print(const param_t * param, int offset, int nodes[], int nodes_count, int verbose, uint32_t ref_timestamp); unsigned int param_list_packed_size(int list_version); int param_list_unpack(int node, void * data, int length, int list_version, int include_remotes); diff --git a/include/param/param_queue.h b/include/param/param_queue.h index 0fc4be9d..54d29c5b 100644 --- a/include/param/param_queue.h +++ b/include/param/param_queue.h @@ -37,7 +37,7 @@ typedef struct param_queue_s { void param_queue_init(param_queue_t * queue, void * buffer, int buffer_size, int used, param_queue_type_e type, int version); -int param_queue_add(param_queue_t *queue, param_t *param, int offset, void *value); +int param_queue_add(param_queue_t *queue, const param_t *param, int offset, void *value); /** * @brief Applies the content of a queue to memory. diff --git a/include/param/param_serializer.h b/include/param/param_serializer.h index 8fb5619a..c0699bea 100644 --- a/include/param/param_serializer.h +++ b/include/param/param_serializer.h @@ -13,10 +13,10 @@ #include #include -void param_serialize_id(mpack_writer_t *writer, param_t * param, int offset, param_queue_t * queue); +void param_serialize_id(mpack_writer_t *writer, const param_t * param, int offset, param_queue_t * queue); void param_deserialize_id(mpack_reader_t *reader, int *id, int *node, csp_timestamp_t *timestamp, int *offset, param_queue_t * queue); -int param_serialize_to_mpack(param_t * param, int offset, mpack_writer_t * writer, void * value, param_queue_t * queue); -void param_deserialize_from_mpack_to_param(void * context, void * queue, param_t * param, int offset, mpack_reader_t * reader); +int param_serialize_to_mpack(const param_t * param, int offset, mpack_writer_t * writer, void * value, param_queue_t * queue); +void param_deserialize_from_mpack_to_param(void * context, void * queue, const param_t * param, int offset, mpack_reader_t * reader); #endif /* SRC_PARAM_PARAM_SERIALIZER_H_ */ diff --git a/include/param/param_string.h b/include/param/param_string.h index 58287737..d5374f02 100644 --- a/include/param/param_string.h +++ b/include/param/param_string.h @@ -2,10 +2,10 @@ #include -void param_value_str(param_t *param, unsigned int i, char * out, int len); +void param_value_str(const param_t *param, unsigned int i, char * out, int len); int param_str_to_value(param_type_e type, char * in, void * out); void param_type_str(param_type_e type, char * out, int len); -void param_print(param_t * param, int offset, int nodes[], int nodes_count, int verbose, uint32_t ref_timestamp); +void param_print(const param_t * param, int offset, int nodes[], int nodes_count, int verbose, uint32_t ref_timestamp); uint32_t param_maskstr_to_mask(const char * str); uint32_t param_umaskstr_to_mask(const char * str); uint32_t param_typestr_to_typeid(const char * str); diff --git a/src/param/collector/param_collector_config.h b/src/param/collector/param_collector_config.h index 730fad8a..d39ec92b 100644 --- a/src/param/collector/param_collector_config.h +++ b/src/param/collector/param_collector_config.h @@ -19,9 +19,9 @@ struct param_collector_config_s { extern struct param_collector_config_s param_collector_config[]; -extern param_t col_run; -extern param_t col_verbose; -extern param_t col_cnfstr; +extern const param_t col_run; +extern const param_t col_verbose; +extern const param_t col_cnfstr; void param_collector_init(void); diff --git a/src/param/list/param_list.c b/src/param/list/param_list.c index c231a3bc..99307ad2 100644 --- a/src/param/list/param_list.c +++ b/src/param/list/param_list.c @@ -28,34 +28,17 @@ #include #endif -/** - * The storage size (i.e. how closely two param_t structs are packed in memory) - * varies from platform to platform (in example on x64 and arm32). This macro - * defines two param_t structs and saves the storage size in a define. - * The linker may also put padding bytes between param_t's (even in the same section), - * but it appears that the same padding is added to the parameters below, so the macro will account for it. - * In addition, Newer GCC versions (gcc 13.3.0 and arm-none-eabi-gcc 13.2.1, common for Ubuntu 24.04) - * will put symbols in reverse order (when compared with gcc 11.4 and arm-none-eabi-gcc 10.3.1, common for Ubuntu 22.04). - * So that necessitates `__attribute__((no_reorder))`, so the size doesn't become negative. - * `__attribute__((no_reorder))` is preferred over c_args '-fno-toplevel-reorder', - * as it doesn't require the user to modify their usage of libparam. - */ -#ifndef PARAM_STORAGE_SIZE -__attribute__((no_reorder)) -const param_t param_size_set0; -__attribute__((no_reorder)) -const param_t param_size_set1; -#define PARAM_STORAGE_SIZE ((intptr_t) ¶m_size_set1 - (intptr_t) ¶m_size_set0) -#endif +#define ALIGN_UP(x, a) (((x) + ((a) - 1)) & ~((a) - 1)) +#define PARAM_STORAGE_SIZE ALIGN_UP(sizeof(param_t), 8) #ifdef PARAM_HAVE_SYS_QUEUE static SLIST_HEAD(param_list_head_s, param_s) param_list_head = {}; #endif -uint8_t param_is_static(param_t * param) { +uint8_t param_is_static(const param_t * param) { - __attribute__((weak)) extern param_t __start_param; - __attribute__((weak)) extern param_t __stop_param; + __attribute__((weak)) extern const param_t __start_param; + __attribute__((weak)) extern const param_t __stop_param; if ((&__start_param != NULL) && (&__start_param != &__stop_param)) { if (param >= &__start_param && param < &__stop_param) @@ -64,14 +47,14 @@ uint8_t param_is_static(param_t * param) { return 0; } -param_t * param_list_iterate(param_list_iterator * iterator) { +const param_t * param_list_iterate(param_list_iterator * iterator) { /** * GNU Linker symbols. These will be autogenerate by GCC when using * __attribute__((section("param")) */ - __attribute__((weak)) extern param_t __start_param; - __attribute__((weak)) extern param_t __stop_param; + __attribute__((weak)) extern const param_t __start_param; + __attribute__((weak)) extern const param_t __stop_param; /* First element */ if (iterator->element == NULL) { @@ -119,7 +102,7 @@ param_t * param_list_iterate(param_list_iterator * iterator) { int param_list_add(param_t * item) { - param_t * param; + const param_t * param; if ((param = param_list_find_id(*item->node, item->id)) != NULL) { /* To protect against updating local static params and ROM remote params @@ -127,11 +110,14 @@ int param_list_add(param_t * item) { strings are readonly. This can be recognized by checking if the VMEM pointer is set */ if (!param_is_static(param) && param->vmem != NULL && param != item) { - param->mask = item->mask; - param->type = item->type; - param->array_size = item->array_size; - param->array_step = item->array_step; - param->vmem->type = item->vmem->type; + + // Explicitly cast away const to allow updating the parameter after checking it resides in dynamic memory + param_t * param_rw = (param_t *) param; + param_rw->mask = item->mask; + param_rw->type = item->type; + param_rw->array_size = item->array_size; + param_rw->array_step = item->array_step; + param_rw->vmem->type = item->vmem->type; if(param->name && item->name){ strcpy(param->name, item->name); @@ -204,13 +190,13 @@ void param_list_remove_specific(param_t * param, uint8_t verbose, int destroy) { } #endif -param_t * param_list_find_id(int node, int id) { +const param_t * param_list_find_id(int node, int id) { if (csp_iflist_get_by_addr(node) != NULL || csp_iflist_get_by_broadcast(node) != NULL) node = 0; - param_t * found = NULL; - param_t * param; + const param_t * found = NULL; + const param_t * param; param_list_iterator i = {}; while ((param = param_list_iterate(&i)) != NULL) { @@ -229,13 +215,13 @@ param_t * param_list_find_id(int node, int id) { return found; } -param_t * param_list_find_name(int node, const char * name) { +const param_t * param_list_find_name(int node, const char * name) { if (node < 0 ) node = 0; - param_t * found = NULL; - param_t * param; + const param_t * found = NULL; + const param_t * param; param_list_iterator i = {}; while ((param = param_list_iterate(&i)) != NULL) { @@ -254,7 +240,7 @@ param_t * param_list_find_name(int node, const char * name) { } void param_list_print(uint32_t mask, int node, const char * globstr, int verbosity) { - param_t * param; + const param_t * param; param_list_iterator i = {}; while ((param = param_list_iterate(&i)) != NULL) { if ((node >= 0) && (*param->node != node)) { @@ -283,7 +269,7 @@ unsigned int param_list_packed_size(int list_version) { int param_list_pack(void* buf, int buf_size, int prio_only, int remote_only, int list_version) { - param_t * param; + const param_t * param; int num_params = 0; void* param_packed = buf; @@ -769,9 +755,9 @@ void param_list_save_wildcard(const char * const filename, int node, int skip_no } } - param_t * param; + const param_t * param; param_list_iterator i = {}; - param_t* param_sorted[1024]; + const param_t* param_sorted[1024]; int param_cnt = 0; while ((param = param_list_iterate(&i)) != NULL) { diff --git a/src/param/param.c b/src/param/param.c index 5577c2f9..1e247586 100644 --- a/src/param/param.c +++ b/src/param/param.c @@ -9,7 +9,7 @@ #define PARAM_GET(_type, _name, _swapfct) \ - _type param_get_##_name##_array(param_t * param, unsigned int i) { \ + _type param_get_##_name##_array(const param_t * param, unsigned int i) { \ if (i >= (unsigned int) param->array_size) { \ return 0; \ } \ @@ -24,7 +24,7 @@ return *(_type *)(param->addr + i * param->array_step); \ } \ } \ - _type param_get_##_name(param_t * param) { \ + _type param_get_##_name(const param_t * param) { \ return param_get_##_name##_array(param, 0); \ } @@ -41,7 +41,7 @@ PARAM_GET(double, double, ) #undef PARAM_GET -void param_get(param_t * param, unsigned int offset, void * value) { +void param_get(const param_t * param, unsigned int offset, void * value) { switch(param->type) { #define PARAM_GET(casename, name, type) \ @@ -76,7 +76,7 @@ void param_get(param_t * param, unsigned int offset, void * value) { } -void param_get_data(param_t * param, void * outbuf, int len) +void param_get_data(const param_t * param, void * outbuf, int len) { if (param->vmem && param->vmem->read) { param->vmem->read(param->vmem, param->vaddr, outbuf, len); @@ -90,7 +90,7 @@ void param_get_data(param_t * param, void * outbuf, int len) #endif #define PARAM_SET(_type, name_in, _swapfct) \ - void __param_set_##name_in(param_t * param, _type value, bool do_callback, unsigned int i) { \ + void __param_set_##name_in(const param_t * param, _type value, bool do_callback, unsigned int i) { \ if (i >= (unsigned int) param->array_size) { \ return; \ } \ @@ -107,19 +107,19 @@ void param_get_data(param_t * param, void * outbuf, int len) param->callback(param, i); \ } \ } \ - inline void param_set_##name_in(param_t * param, _type value) \ + inline void param_set_##name_in(const param_t * param, _type value) \ { \ __param_set_##name_in(param, value, true, 0); \ } \ - inline void param_set_##name_in##_nocallback(param_t * param, _type value) \ + inline void param_set_##name_in##_nocallback(const param_t * param, _type value) \ { \ __param_set_##name_in(param, value, false, 0); \ } \ - inline void param_set_##name_in##_array(param_t * param, unsigned int i, _type value) \ + inline void param_set_##name_in##_array(const param_t * param, unsigned int i, _type value) \ { \ __param_set_##name_in(param, value, true, i); \ } \ - inline void param_set_##name_in##_array_nocallback(param_t * param, unsigned int i, _type value) \ + inline void param_set_##name_in##_array_nocallback(const param_t * param, unsigned int i, _type value) \ { \ __param_set_##name_in(param, value, false, i); \ } @@ -137,7 +137,7 @@ PARAM_SET(double, double, ) #undef PARAM_SET -void param_set(param_t * param, unsigned int offset, void * value) { +void param_set(const param_t * param, unsigned int offset, void * value) { switch(param->type) { #define PARAM_SET(casename, name, type) \ @@ -170,7 +170,7 @@ void param_set(param_t * param, unsigned int offset, void * value) { } } -void param_set_string(param_t * param, const char * inbuf, int len) { +void param_set_string(const param_t * param, const char * inbuf, int len) { param_set_data_nocallback(param, inbuf, len); /* Termination */ if (param->vmem && param->vmem->write) { @@ -184,7 +184,7 @@ void param_set_string(param_t * param, const char * inbuf, int len) { } } -void param_set_data_nocallback(param_t * param, const void * inbuf, int len) { +void param_set_data_nocallback(const param_t * param, const void * inbuf, int len) { if (param->vmem && param->vmem->write) { param->vmem->write(param->vmem, param->vaddr, inbuf, len); } else { @@ -192,7 +192,7 @@ void param_set_data_nocallback(param_t * param, const void * inbuf, int len) { } } -void param_set_data(param_t * param, const void * inbuf, int len) { +void param_set_data(const param_t * param, const void * inbuf, int len) { param_set_data_nocallback(param, inbuf, len); /* Callback */ if (param->callback) { @@ -223,7 +223,7 @@ int param_typesize(param_type_e type) { return -1; } -int param_size(param_t * param) { +int param_size(const param_t * param) { switch(param->type) { case PARAM_TYPE_STRING: case PARAM_TYPE_DATA: @@ -233,7 +233,7 @@ int param_size(param_t * param) { } } -void param_copy(param_t * dest, param_t * src) { +void param_copy(const param_t * dest, const param_t * src) { /* Type check */ if (dest->type != src->type) { diff --git a/src/param/param_client.c b/src/param/param_client.c index c4881695..7388daf9 100644 --- a/src/param/param_client.c +++ b/src/param/param_client.c @@ -46,7 +46,7 @@ static void param_transaction_callback_pull(csp_packet_t *response, int verbose, param_deserialize_id(&reader, &id, &node, ×tamp, &offset, &queue); if (node == 0) node = response->id.src; - param_t * param = param_list_find_id(node, id); + const param_t * param = param_list_find_id(node, id); /* We need to discard the data field, to get to next paramid */ mpack_discard (&reader); diff --git a/src/param/param_queue.c b/src/param/param_queue.c index b57f5436..8bed592d 100644 --- a/src/param/param_queue.c +++ b/src/param/param_queue.c @@ -27,7 +27,7 @@ int id, node, offset = -1; \ csp_timestamp_t timestamp = { .tv_sec = 0, .tv_nsec = 0 }; \ param_deserialize_id(&reader, &id, &node, ×tamp, &offset, queue); \ - param_t * param = param_list_find_id(node, id); \ + const param_t * param = param_list_find_id(node, id); \ void param_queue_init(param_queue_t *queue, void *buffer, int buffer_size, int used, param_queue_type_e type, int version) { queue->buffer = buffer; @@ -41,7 +41,7 @@ void param_queue_init(param_queue_t *queue, void *buffer, int buffer_size, int u queue->client_timestamp.tv_nsec = 0; } -int param_queue_add(param_queue_t *queue, param_t *param, int offset, void *value) { +int param_queue_add(param_queue_t *queue, const param_t *param, int offset, void *value) { /* Ensure we always send nodeid on the first element of the queue */ if (queue->used == 0) { @@ -88,7 +88,7 @@ int param_queue_apply(param_queue_t *queue, int host, int verbose) { node = host; /* Search on the specified node in the request or response */ - param_t * param = param_list_find_id(node, id); + const param_t * param = param_list_find_id(node, id); if (param) { if ((param->mask & PM_ATOMIC_WRITE) && (atomic_write == 0)) { @@ -218,7 +218,7 @@ void param_queue_print_params(param_queue_t *queue, uint32_t ref_timestamp) { int _id, _node, _offset = -1; csp_timestamp_t _timestamp = { .tv_sec = 0, .tv_nsec = 0 }; param_deserialize_id(&_reader, &_id, &_node, &_timestamp, &_offset, queue); - param_t * _param = param_list_find_id(_node, _id); + const param_t * _param = param_list_find_id(_node, _id); if(queue->type == PARAM_QUEUE_TYPE_SET){ mpack_discard(&_reader); } diff --git a/src/param/param_serializer.c b/src/param/param_serializer.c index 8a427515..a966ebc6 100644 --- a/src/param/param_serializer.c +++ b/src/param/param_serializer.c @@ -34,7 +34,7 @@ static const uint16_t known_header_mask = (1 << PARAM_HEADER_ARRAY_POS) | (1 << PARAM_HEADER_EXTENDEDTIMESTAMP_POS) | PARAM_HEADER_ID_MASK; -static inline uint16_t param_get_short_id(param_t * param, unsigned int isarray, unsigned int reserved) { +static inline uint16_t param_get_short_id(const param_t * param, unsigned int isarray, unsigned int reserved) { uint16_t node = *param->node; return (node << 11) | ((isarray & 0x1) << 10) | ((reserved & 0x1) << 2) | ((param->id) & 0x1FF); } @@ -51,7 +51,7 @@ static inline uint16_t param_parse_short_id_paramid(uint16_t short_id) { return short_id & 0x1FF; } -void param_serialize_id(mpack_writer_t *writer, param_t *param, int offset, param_queue_t *queue) { +void param_serialize_id(mpack_writer_t *writer, const param_t *param, int offset, param_queue_t *queue) { if (queue->version == 1) { @@ -198,7 +198,7 @@ void param_deserialize_id(mpack_reader_t *reader, int *id, int *node, csp_timest } -int param_serialize_to_mpack(param_t * param, int offset, mpack_writer_t * writer, void * value, param_queue_t * queue) { +int param_serialize_to_mpack(const param_t * param, int offset, mpack_writer_t * writer, void * value, param_queue_t * queue) { /* Remember the initial position if we need to abort later due to buffer full */ char * init_pos = writer->position; @@ -380,7 +380,7 @@ int param_serialize_to_mpack(param_t * param, int offset, mpack_writer_t * write } -void param_deserialize_from_mpack_to_param(void * context, void * queue, param_t * param, int offset, mpack_reader_t * reader) { +void param_deserialize_from_mpack_to_param(void * context, void * queue, const param_t * param, int offset, mpack_reader_t * reader) { if (offset < 0) offset = 0; diff --git a/src/param/param_server.c b/src/param/param_server.c index 66cfe17c..9b16ba37 100644 --- a/src/param/param_server.c +++ b/src/param/param_server.c @@ -68,7 +68,7 @@ static void __send(struct param_serve_context *ctx, int end) { csp_sendto_reply(ctx->request, ctx->response, CSP_O_SAME); } -static int __add(struct param_serve_context *ctx, param_t * param, int offset) { +static int __add(struct param_serve_context *ctx, const param_t * param, int offset) { int result = param_queue_add(&ctx->q_response, param, offset, NULL); if (result != 0) { @@ -113,7 +113,7 @@ static void param_serve_pull_request(csp_packet_t * request, int all, int versio int id, node, offset = -1; csp_timestamp_t timestamp = { .tv_sec = 0, .tv_nsec = 0 }; param_deserialize_id(&reader, &id, &node, ×tamp, &offset, &q_request); - param_t * param = param_list_find_id(node, id); + const param_t * param = param_list_find_id(node, id); if (param) { if(ack_with_pull) { /* Move reader forward to skip values as we normally use a get queue */ @@ -127,7 +127,7 @@ static void param_serve_pull_request(csp_packet_t * request, int all, int versio int _id, _node, _offset = -1; csp_timestamp_t _timestamp = { .tv_sec = 0, .tv_nsec = 0 }; param_deserialize_id(&_reader, &_id, &_node, &_timestamp, &_offset, &ctx.q_response); - param_t * _param = param_list_find_id(_node, _id); + const param_t * _param = param_list_find_id(_node, _id); /* Move reader forward to skip values */ mpack_discard(&_reader); @@ -160,7 +160,7 @@ static void param_serve_pull_request(csp_packet_t * request, int all, int versio } else { /* Loop the full parameter list */ - param_t * param; + const param_t * param; param_list_iterator i = {}; while ((param = param_list_iterate(&i)) != NULL) { if (param->mask & PM_HIDDEN) { diff --git a/src/param/param_string.c b/src/param/param_string.c index af39f431..c6477186 100644 --- a/src/param/param_string.c +++ b/src/param/param_string.c @@ -37,7 +37,7 @@ static int nibble(char c) { return -1; } -void param_value_str(param_t *param, unsigned int i, char * out, int len) +void param_value_str(const param_t *param, unsigned int i, char * out, int len) { switch (param->type) { #define PARAM_SWITCH_SNPRINTF(casename, strtype, strcast, name) \ @@ -222,7 +222,7 @@ void param_type_str(param_type_e type, char * out, int len) } } -static void param_print_value(FILE * file, param_t * param, int offset) { +static void param_print_value(FILE * file, const param_t * param, int offset) { if (param == NULL) { return; @@ -291,7 +291,7 @@ static void param_print_value(FILE * file, param_t * param, int offset) { } -void param_print_file(FILE* file, param_t * param, int offset, int nodes[], int nodes_count, int verbose, uint32_t ref_timestamp) +void param_print_file(FILE* file, const param_t * param, int offset, int nodes[], int nodes_count, int verbose, uint32_t ref_timestamp) { if (param == NULL) return; @@ -318,7 +318,7 @@ void param_print_file(FILE* file, param_t * param, int offset, int nodes[], int /* Value table */ if (nodes_count > 0 && nodes != NULL) { for(int i = 0; i < nodes_count; i++) { - param_t * specific_param = param_list_find_id(nodes[i], param->id); + const param_t * specific_param = param_list_find_id(nodes[i], param->id); param_print_value(file, specific_param, offset); } @@ -444,7 +444,7 @@ void param_print_file(FILE* file, param_t * param, int offset, int nodes[], int } -void param_print(param_t * param, int offset, int nodes[], int nodes_count, int verbose, uint32_t ref_timestamp) { +void param_print(const param_t * param, int offset, int nodes[], int nodes_count, int verbose, uint32_t ref_timestamp) { param_print_file(stdout, param, offset, nodes, nodes_count, verbose, ref_timestamp); } diff --git a/src/vmem/vmem_server.c b/src/vmem/vmem_server.c index 96c73862..de90d388 100644 --- a/src/vmem/vmem_server.c +++ b/src/vmem/vmem_server.c @@ -291,7 +291,7 @@ void vmem_server_handler(csp_conn_t * conn) static void rparam_list_handler(csp_conn_t * conn) { - param_t * param; + const param_t * param; param_list_iterator i = {}; while ((param = param_list_iterate(&i)) != NULL) { if (param->mask & PM_HIDDEN) { From 16f05e371e14c8acc44499f3ae7f95a2a883b867 Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Wed, 28 Jan 2026 15:29:35 +0100 Subject: [PATCH 02/11] Make vmem const --- include/param/param.h | 2 +- include/vmem/vmem.h | 10 +++++----- include/vmem/vmem_block.h | 4 ++-- include/vmem/vmem_file.h | 8 ++++---- include/vmem/vmem_fram.h | 8 ++++---- include/vmem/vmem_fram_cache.h | 8 ++++---- include/vmem/vmem_mmap.h | 4 ++-- include/vmem/vmem_ram.h | 8 ++++---- src/param/collector/param_collector_config.c | 2 +- src/param/list/param_list.c | 2 +- src/vmem/vmem.c | 4 ++-- src/vmem/vmem_fram.c | 4 ++-- src/vmem/vmem_fram_cache.c | 4 ++-- src/vmem/vmem_internal.h | 4 ++-- 14 files changed, 36 insertions(+), 36 deletions(-) diff --git a/include/param/param.h b/include/param/param.h index 63c33000..b5855818 100644 --- a/include/param/param.h +++ b/include/param/param.h @@ -105,7 +105,7 @@ typedef struct param_s { /* Storage */ void * addr; /* Physical address */ uint64_t vaddr; /* Virtual address in case of VMEM */ - struct vmem_s * vmem; + const struct vmem_s * vmem; int array_size; int array_step; diff --git a/include/vmem/vmem.h b/include/vmem/vmem.h index 100cef03..ee7bfe1a 100644 --- a/include/vmem/vmem.h +++ b/include/vmem/vmem.h @@ -35,9 +35,9 @@ enum vmem_types{ typedef struct vmem_s { int type; - void (*read)(struct vmem_s * vmem, uint64_t addr, void * dataout, uint32_t len); - void (*write)(struct vmem_s * vmem, uint64_t addr, const void * datain, uint32_t len); - int (*flush)(struct vmem_s * vmem); + void (*read)(const struct vmem_s * vmem, uint64_t addr, void * dataout, uint32_t len); + void (*write)(const struct vmem_s * vmem, uint64_t addr, const void * datain, uint32_t len); + int (*flush)(const struct vmem_s * vmem); /* This anonymous union is needed to be able to handle 64-bit and 32-bit * systems interchangeably. Since the VMEM backend always expects 64-bit * vaddr, and we are not able to initialize the 64-bit vaddr field with @@ -152,11 +152,11 @@ void vmem_add(vmem_t * start, vmem_t * stop); /** * @brief linker-generated symbol for the first VMEM element in the linker "vmem" section */ -extern vmem_t __start_vmem __attribute__((weak)); +extern const vmem_t __start_vmem __attribute__((weak)); /** * @brief linker-generated symbol for the last VMEM element in the linker "vmem" section */ -extern vmem_t __stop_vmem __attribute__((weak)); +extern const vmem_t __stop_vmem __attribute__((weak)); #ifdef __cplusplus } diff --git a/include/vmem/vmem_block.h b/include/vmem/vmem_block.h index 79b068a7..222d4f4e 100644 --- a/include/vmem/vmem_block.h +++ b/include/vmem/vmem_block.h @@ -113,9 +113,9 @@ typedef struct vmem_block_region_s { .options = options_in, \ }; \ __attribute__((section("vmem"))) \ - __attribute__((__aligned__(__alignof__(vmem_t)))) \ + __attribute__((__aligned__(8))) \ __attribute__((used)) \ - vmem_t vmem_##name_in = { \ + const vmem_t vmem_##name_in = { \ .type = VMEM_TYPE_BLOCK, \ .read = vmem_block_read, \ .write = vmem_block_write, \ diff --git a/include/vmem/vmem_file.h b/include/vmem/vmem_file.h index 0fd6d71c..af783a97 100644 --- a/include/vmem/vmem_file.h +++ b/include/vmem/vmem_file.h @@ -43,9 +43,9 @@ void vmem_file_write(vmem_t * vmem, uint64_t addr, const void * datain, uint32_t .filename = filename_in, \ }; \ __attribute__((section("vmem"))) \ - __attribute__((aligned(__alignof__(vmem_t)))) \ + __attribute__((aligned(8))) \ __attribute__((used)) \ - vmem_t vmem_##name_in = { \ + const vmem_t vmem_##name_in = { \ .type = VMEM_TYPE_FILE, \ .name = strname, \ .size = size_in, \ @@ -63,9 +63,9 @@ void vmem_file_write(vmem_t * vmem, uint64_t addr, const void * datain, uint32_t .filename = filename_in, \ }; \ __attribute__((section("vmem"))) \ - __attribute__((aligned(__alignof__(vmem_t)))) \ + __attribute__((aligned(8))) \ __attribute__((used)) \ - vmem_t vmem_##name_in = { \ + const vmem_t vmem_##name_in = { \ .type = VMEM_TYPE_FILE, \ .name = strname, \ .size = size_in, \ diff --git a/include/vmem/vmem_fram.h b/include/vmem/vmem_fram.h index 7b76cfd9..61fb79ad 100644 --- a/include/vmem/vmem_fram.h +++ b/include/vmem/vmem_fram.h @@ -19,9 +19,9 @@ typedef struct { .fram_addr = fram_addr_in, \ }; \ __attribute__((section("vmem"))) \ - __attribute__((__aligned__(__alignof__(vmem_t)))) \ + __attribute__((__aligned__(8))) \ __attribute__((used)) \ - vmem_t vmem_##name_in = { \ + const vmem_t vmem_##name_in = { \ .type = VMEM_TYPE_FRAM, \ .name = strname, \ .size = size_in, \ @@ -32,8 +32,8 @@ typedef struct { .ack_with_pull = 1, \ }; -void vmem_fram_read(vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len); -void vmem_fram_write(vmem_t * vmem, uint64_t addr, const void * datain, uint32_t len); +void vmem_fram_read(const vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len); +void vmem_fram_write(const vmem_t * vmem, uint64_t addr, const void * datain, uint32_t len); #endif /* SRC_PARAM_VMEM_FRAM_H_ */ diff --git a/include/vmem/vmem_fram_cache.h b/include/vmem/vmem_fram_cache.h index 89214aae..3f577c05 100644 --- a/include/vmem/vmem_fram_cache.h +++ b/include/vmem/vmem_fram_cache.h @@ -28,9 +28,9 @@ typedef struct { .cache_status = 0, \ }; \ __attribute__((section("vmem"))) \ - __attribute__((__aligned__(__alignof__(vmem_t)))) \ + __attribute__((__aligned__(8))) \ __attribute__((used)) \ - vmem_t vmem_##name_in = { \ + const vmem_t vmem_##name_in = { \ .type = VMEM_TYPE_FRAM_CACHE, \ .name = strname, \ .size = size_in, \ @@ -41,5 +41,5 @@ typedef struct { .ack_with_pull = 1, \ }; -void vmem_fram_cache_read(vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len); -void vmem_fram_cache_write(vmem_t * vmem, uint64_t addr, const void * datain, uint32_t len); +void vmem_fram_cache_read(const vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len); +void vmem_fram_cache_write(const vmem_t * vmem, uint64_t addr, const void * datain, uint32_t len); diff --git a/include/vmem/vmem_mmap.h b/include/vmem/vmem_mmap.h index ef55247e..46d4b928 100644 --- a/include/vmem/vmem_mmap.h +++ b/include/vmem/vmem_mmap.h @@ -20,9 +20,9 @@ void vmem_mmap_write(vmem_t * vmem, uint64_t addr, const void * datain, uint32_t .filename = filename_in, \ }; \ __attribute__((section("vmem"))) \ - __attribute__((__aligned__(__alignof__(vmem_t)))) \ + __attribute__((__aligned__(8))) \ __attribute__((used)) \ - vmem_t vmem_mmap_##name_in = { \ + const vmem_t vmem_mmap_##name_in = { \ .type = VMEM_TYPE_FILE, \ .name = strname, \ .size = size_in, \ diff --git a/include/vmem/vmem_ram.h b/include/vmem/vmem_ram.h index 80c5ac16..6545fc34 100644 --- a/include/vmem/vmem_ram.h +++ b/include/vmem/vmem_ram.h @@ -40,9 +40,9 @@ typedef struct { .physaddr = vmem_##name_in##_heap, \ }; \ __attribute__((section("vmem"))) \ - __attribute__((__aligned__(__alignof__(vmem_t)))) \ + __attribute__((__aligned__(8))) \ __attribute__((used)) \ - vmem_t vmem_##name_in = { \ + const vmem_t vmem_##name_in = { \ .type = VMEM_TYPE_RAM, \ .read = NULL, \ .write = NULL, \ @@ -67,9 +67,9 @@ typedef struct { .physaddr = mem_addr, \ }; \ __attribute__((section("vmem"))) \ - __attribute__((__aligned__(__alignof__(vmem_t)))) \ + __attribute__((__aligned__(8))) \ __attribute__((used)) \ - vmem_t vmem_##name_in = { \ + const vmem_t vmem_##name_in = { \ .type = VMEM_TYPE_RAM, \ .read = NULL, \ .write = NULL, \ diff --git a/src/param/collector/param_collector_config.c b/src/param/collector/param_collector_config.c index 7835707d..ebe11a2c 100644 --- a/src/param/collector/param_collector_config.c +++ b/src/param/collector/param_collector_config.c @@ -22,7 +22,7 @@ void param_col_confstr_callback(struct param_s * param, int offset) { param_collector_init(); } -extern vmem_t vmem_col; +extern const vmem_t vmem_col; PARAM_DEFINE_STATIC_VMEM(PARAMID_COLLECTOR_RUN, col_run, PARAM_TYPE_UINT8, 0, sizeof(uint8_t), PM_CONF, NULL, "", col, 0x0, "Internal use"); PARAM_DEFINE_STATIC_VMEM(PARAMID_COLLECTOR_VERBOSE, col_verbose, PARAM_TYPE_UINT8, 0, sizeof(uint8_t), PM_CONF, NULL, "", col, 0x1, "Internal use"); PARAM_DEFINE_STATIC_VMEM(PARAMID_COLLECTOR_CNFSTR, col_cnfstr, PARAM_TYPE_STRING, 100, 0, PM_CONF, param_col_confstr_callback, "", col, 0x02, "Internal use"); diff --git a/src/param/list/param_list.c b/src/param/list/param_list.c index 99307ad2..f727cd28 100644 --- a/src/param/list/param_list.c +++ b/src/param/list/param_list.c @@ -117,7 +117,7 @@ int param_list_add(param_t * item) { param_rw->type = item->type; param_rw->array_size = item->array_size; param_rw->array_step = item->array_step; - param_rw->vmem->type = item->vmem->type; + //param_rw->vmem->type = item->vmem->type; // TODO do not support overwriting vmem type if(param->name && item->name){ strcpy(param->name, item->name); diff --git a/src/vmem/vmem.c b/src/vmem/vmem.c index 74102817..761baa94 100644 --- a/src/vmem/vmem.c +++ b/src/vmem/vmem.c @@ -22,7 +22,7 @@ void * vmem_memcpy(void * to, const void * from, uint32_t size) { return vmem_cpy((uint64_t)(uintptr_t)to, (uint64_t)(uintptr_t)from, (uint64_t)size); } -void * vmem_write_direct(vmem_t * vmem, uint64_t to, const void * from, uint32_t size) { +void * vmem_write_direct(const vmem_t * vmem, uint64_t to, const void * from, uint32_t size) { /* Write to VMEM */ if ((to >= vmem->vaddr) && (to + (uint64_t)size <= vmem->vaddr + vmem->size)) { @@ -36,7 +36,7 @@ void * vmem_write_direct(vmem_t * vmem, uint64_t to, const void * from, uint32_t return NULL; } -void * vmem_read_direct(vmem_t * vmem, void * to, uint64_t from, uint32_t size) { +void * vmem_read_direct(const vmem_t * vmem, void * to, uint64_t from, uint32_t size) { /* Read */ if ((from >= vmem->vaddr) && (from + (uint64_t)size <= vmem->vaddr + vmem->size)) { diff --git a/src/vmem/vmem_fram.c b/src/vmem/vmem_fram.c index 1cd14358..1386aaec 100644 --- a/src/vmem/vmem_fram.c +++ b/src/vmem/vmem_fram.c @@ -18,11 +18,11 @@ void fram_write_data(uint32_t addr, const void *data, uint32_t len); void fram_read_data(uint32_t addr, void *data, uint32_t len); -void vmem_fram_read(vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len) { +void vmem_fram_read(const vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len) { fram_read_data(((uintptr_t) ((vmem_fram_driver_t*) vmem->driver)->fram_addr) + (uintptr_t)addr, dataout, len); } -void vmem_fram_write(vmem_t * vmem, uint64_t addr, const void * datain, uint32_t len) { +void vmem_fram_write(const vmem_t * vmem, uint64_t addr, const void * datain, uint32_t len) { fram_write_data(((uintptr_t) ((vmem_fram_driver_t*) vmem->driver)->fram_addr) + (uintptr_t)addr, datain, len); } diff --git a/src/vmem/vmem_fram_cache.c b/src/vmem/vmem_fram_cache.c index baca6799..7d7f36bc 100644 --- a/src/vmem/vmem_fram_cache.c +++ b/src/vmem/vmem_fram_cache.c @@ -18,7 +18,7 @@ void fram_write_data(uint32_t addr, const void *data, uint32_t len); void fram_read_data(uint32_t addr, void *data, uint32_t len); -void vmem_fram_cache_read(vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len) { +void vmem_fram_cache_read(const vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len) { vmem_fram_cache_driver_t * driver = vmem->driver; @@ -34,7 +34,7 @@ void vmem_fram_cache_read(vmem_t * vmem, uint64_t addr, void * dataout, uint32_t } -void vmem_fram_cache_write(vmem_t * vmem, uint64_t addr, const void * datain, uint32_t len) { +void vmem_fram_cache_write(const vmem_t * vmem, uint64_t addr, const void * datain, uint32_t len) { vmem_fram_cache_driver_t * driver = vmem->driver; diff --git a/src/vmem/vmem_internal.h b/src/vmem/vmem_internal.h index fc2e8644..6608f98a 100644 --- a/src/vmem/vmem_internal.h +++ b/src/vmem/vmem_internal.h @@ -37,7 +37,7 @@ vmem_t *vmem_from_iter(vmem_iter_t * iter); * @param size Number of bytes to transfer * @return void* always NULL, no error detection possible at this time */ -void * vmem_write_direct(vmem_t * vmem, uint64_t to, const void * from, uint32_t size); +void * vmem_write_direct(const vmem_t * vmem, uint64_t to, const void * from, uint32_t size); /** * @brief Read chunk of data from VMEM to physical memory @@ -47,4 +47,4 @@ void * vmem_write_direct(vmem_t * vmem, uint64_t to, const void * from, uint32_t * @param size Number of bytes to transfer * @return void* always NULL, no error detection possible at this time */ -void * vmem_read_direct(vmem_t * vmem, void * to, uint64_t from, uint32_t size); +void * vmem_read_direct(const vmem_t * vmem, void * to, uint64_t from, uint32_t size); From 602d7b11ccdf860ea7865da6bd0c5d73a532e059 Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Thu, 29 Jan 2026 11:44:56 +0100 Subject: [PATCH 03/11] Avoid function scope statics These are given funky names by gcc, like csp_buffer_pool.2, which make them difficult for static analysis tools to find and put into context. Moving to file scope is only a slight scope increase which can be tolerated for the benefits of static analysis. --- src/vmem/vmem_server.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vmem/vmem_server.c b/src/vmem/vmem_server.c index de90d388..c75b6e2b 100644 --- a/src/vmem/vmem_server.c +++ b/src/vmem/vmem_server.c @@ -23,6 +23,9 @@ #include #include "vmem_internal.h" +/* Statically allocate a listener socket */ +static csp_socket_t vmem_server_socket = {0}; + #ifdef PARAM_LIST_DYNAMIC SLIST_HEAD(vmem_handler_obj_list_s, vmem_handler_obj_s); @@ -334,9 +337,6 @@ static void rparam_list_handler(csp_conn_t * conn) void vmem_server_loop(void * param) { - /* Statically allocate a listener socket */ - static csp_socket_t vmem_server_socket = {0}; - /* Bind all ports to socket */ csp_bind(&vmem_server_socket, VMEM_PORT_SERVER); csp_bind(&vmem_server_socket, PARAM_PORT_LIST); From 122c7c48718f8d3e068984d26be32a9c862e8e99 Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Fri, 30 Jan 2026 20:23:58 +0100 Subject: [PATCH 04/11] force local statics to use address zero For remote parameters use param_define_remote instead --- include/param/param.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/include/param/param.h b/include/param/param.h index b5855818..d4f0bf20 100644 --- a/include/param/param.h +++ b/include/param/param.h @@ -149,16 +149,16 @@ typedef struct param_s { #define PARAM_TIMESTAMP_INIT(_name) #endif +static const uint16_t node_self = 0; #define PARAM_DEFINE_STATIC_RAM(_id, _name, _type, _array_count, _array_step, _flags, _callback, _unit, _physaddr, _docstr) \ ; /* Catch const param defines */ \ PARAM_TIMESTAMP_DECL(_name) \ - uint16_t _node_##_name = 0; \ __attribute__((section("param"))) \ __attribute__((used, no_reorder, aligned(8))) \ const param_t _name = { \ .vmem = NULL, \ - .node = &_node_##_name, \ + .node = (uint16_t *) &node_self, \ .id = _id, \ .type = _type, \ .name = #_name, \ @@ -176,11 +176,10 @@ typedef struct param_s { #define PARAM_DEFINE_STATIC_VMEM(_id, _name, _type, _array_count, _array_step, _flags, _callback, _unit, _vmem_name, _vmem_addr, _docstr) \ ; /* Catch const param defines */ \ PARAM_TIMESTAMP_DECL(_name) \ - uint16_t _node_##_name = 0; \ __attribute__((section("param"))) \ __attribute__((used, no_reorder, aligned(8))) \ const param_t _name = { \ - .node = &_node_##_name, \ + .node = (uint16_t *) &node_self, \ .id = _id, \ .type = _type, \ .name = #_name, \ From 1a7196dc62789a29ff394b7dfc3825790952b10b Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Sun, 1 Feb 2026 17:51:11 +0100 Subject: [PATCH 05/11] stop using no_reorder which is an unknown attribute also remove include vmem.h as its not used --- include/param/param.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/include/param/param.h b/include/param/param.h index d4f0bf20..d136f4b6 100644 --- a/include/param/param.h +++ b/include/param/param.h @@ -10,7 +10,6 @@ #include #include -#include #include @@ -155,7 +154,7 @@ static const uint16_t node_self = 0; ; /* Catch const param defines */ \ PARAM_TIMESTAMP_DECL(_name) \ __attribute__((section("param"))) \ - __attribute__((used, no_reorder, aligned(8))) \ + __attribute__((used, aligned(8))) \ const param_t _name = { \ .vmem = NULL, \ .node = (uint16_t *) &node_self, \ @@ -177,7 +176,7 @@ static const uint16_t node_self = 0; ; /* Catch const param defines */ \ PARAM_TIMESTAMP_DECL(_name) \ __attribute__((section("param"))) \ - __attribute__((used, no_reorder, aligned(8))) \ + __attribute__((used, aligned(8))) \ const param_t _name = { \ .node = (uint16_t *) &node_self, \ .id = _id, \ @@ -201,7 +200,7 @@ static const uint16_t node_self = 0; ; /* Catch const param defines */ \ PARAM_TIMESTAMP_DECL(_name) \ __attribute__((section("param"))) \ - __attribute__((used, no_reorder, aligned(8))) \ + __attribute__((used, aligned(8))) \ const param_t _name = { \ .node = _nodeaddr, \ .id = _id, \ From db59e0686a64572d7978734f5c02a1745087f1b3 Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Mon, 2 Feb 2026 10:13:46 +0100 Subject: [PATCH 06/11] Reorder param_t for more efficient storage --- include/param/param.h | 33 ++++++++++++++++----------------- src/param/list/param_list.c | 1 + 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/param/param.h b/include/param/param.h index d136f4b6..61c2ced8 100644 --- a/include/param/param.h +++ b/include/param/param.h @@ -89,37 +89,36 @@ typedef enum { /** * Parameter description structure * Note: this is not packed in order to maximise run-time efficiency + * But the order of the elements is chosen to minimize padding. + * So we start by largest types first, down to smallest types. */ typedef struct param_s { + + uint64_t vaddr; /* Virtual address in case of VMEM */ - /* Parameter declaration */ - uint16_t id; uint16_t * node; - param_type_e type; - uint32_t mask; char *name; char *unit; char *docstr; - - /* Storage */ void * addr; /* Physical address */ - uint64_t vaddr; /* Virtual address in case of VMEM */ const struct vmem_s * vmem; - int array_size; - int array_step; - - /* Local info */ void (*callback)(const struct param_s * param, int offset); -#ifdef PARAM_HAVE_TIMESTAMP - csp_timestamp_t * timestamp; -#endif -#ifdef PARAM_HAVE_SYS_QUEUE + #ifdef PARAM_HAVE_TIMESTAMP + csp_timestamp_t * timestamp; + #endif + + #ifdef PARAM_HAVE_SYS_QUEUE /* single linked list: - * The weird definition format comes from sys/queue.h SLINST_ENTRY() macro */ + * The weird definition format comes from sys/queue.h SLINST_ENTRY() macro */ struct { struct param_s *sle_next; } next; -#endif + #endif + uint32_t mask; + uint16_t id; + uint16_t array_step; // Deliberate use of 16-bit to balance speed and size + uint16_t array_size; // Deliberate use of 16-bit to balance speed and size + uint16_t type; // Deliberate use of 16-bit to balance speed and size } param_t; diff --git a/src/param/list/param_list.c b/src/param/list/param_list.c index f727cd28..c1a908c8 100644 --- a/src/param/list/param_list.c +++ b/src/param/list/param_list.c @@ -30,6 +30,7 @@ #define ALIGN_UP(x, a) (((x) + ((a) - 1)) & ~((a) - 1)) #define PARAM_STORAGE_SIZE ALIGN_UP(sizeof(param_t), 8) +_Static_assert(__alignof__(param_t) == 8, "param_t alignment must be exactly 8 bytes"); #ifdef PARAM_HAVE_SYS_QUEUE static SLIST_HEAD(param_list_head_s, param_s) param_list_head = {}; From bf5158414332e6ee02be2f7b12942eb65bfd9659 Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Mon, 2 Feb 2026 11:30:10 +0100 Subject: [PATCH 07/11] param client const fix --- include/param/param_client.h | 4 ++-- src/param/param_client.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/param/param_client.h b/include/param/param_client.h index ada01fad..2e31362a 100644 --- a/include/param/param_client.h +++ b/include/param/param_client.h @@ -35,7 +35,7 @@ * @param version 1 or 2 * @return 0 = ok, -1 on network error */ -int param_pull_single(param_t *param, int offset, int prio, int verbose, int host, int timeout, int version); +int param_pull_single(const param_t *param, int offset, int prio, int verbose, int host, int timeout, int version); /** * PULL all @@ -65,7 +65,7 @@ int param_pull_all(int prio, int verbose, int host, uint32_t include_mask, uint3 * @param ack_with_pull ack with param queue * @return 0 = OK, -1 on network error */ -int param_push_single(param_t *param, int offset, int prio, void *value, int verbose, int host, int timeout, int version, bool ack_with_pull); +int param_push_single(const param_t *param, int offset, int prio, void *value, int verbose, int host, int timeout, int version, bool ack_with_pull); /** * QUEUE PARAMETER API diff --git a/src/param/param_client.c b/src/param/param_client.c index 7388daf9..d5db22e7 100644 --- a/src/param/param_client.c +++ b/src/param/param_client.c @@ -165,7 +165,7 @@ int param_pull_queue(param_queue_t *queue, uint8_t prio, int verbose, int host, } -int param_pull_single(param_t *param, int offset, int prio, int verbose, int host, int timeout, int version) { +int param_pull_single(const param_t *param, int offset, int prio, int verbose, int host, int timeout, int version) { csp_packet_t * packet = csp_buffer_get(PARAM_SERVER_MTU); if (packet == NULL) @@ -245,7 +245,7 @@ int param_push_queue(param_queue_t *queue, int prio, int verbose, int host, int return 0; } -int param_push_single(param_t *param, int offset, int prio, void *value, int verbose, int host, int timeout, int version, bool ack_with_pull) { +int param_push_single(const param_t *param, int offset, int prio, void *value, int verbose, int host, int timeout, int version, bool ack_with_pull) { csp_packet_t * packet = csp_buffer_get(PARAM_SERVER_MTU); if (packet == NULL) From 43e16818e37417bb6e0ae644a2cbe0e60b934847 Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Mon, 2 Feb 2026 11:31:15 +0100 Subject: [PATCH 08/11] Do not export __start_vmem This conflicts with some users having const region and others non const region --- include/vmem/vmem.h | 9 --------- src/vmem/vmem.c | 3 +++ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/include/vmem/vmem.h b/include/vmem/vmem.h index ee7bfe1a..8d4e10a8 100644 --- a/include/vmem/vmem.h +++ b/include/vmem/vmem.h @@ -149,15 +149,6 @@ int vmem_flush(vmem_t *vmem); */ void vmem_add(vmem_t * start, vmem_t * stop); -/** - * @brief linker-generated symbol for the first VMEM element in the linker "vmem" section - */ -extern const vmem_t __start_vmem __attribute__((weak)); -/** - * @brief linker-generated symbol for the last VMEM element in the linker "vmem" section - */ -extern const vmem_t __stop_vmem __attribute__((weak)); - #ifdef __cplusplus } #endif diff --git a/src/vmem/vmem.c b/src/vmem/vmem.c index 761baa94..f11d62b3 100644 --- a/src/vmem/vmem.c +++ b/src/vmem/vmem.c @@ -163,6 +163,9 @@ int vmem_ptr_to_index(vmem_t * vmem) { return -1; } +extern const vmem_t __start_vmem __attribute__((weak)); +extern const vmem_t __stop_vmem __attribute__((weak)); + #ifdef PARAM_LIST_DYNAMIC static vmem_iter_t g_start = { #else From a77279d9f9281e6e593190c74d18c7e1d2b01860 Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Mon, 2 Feb 2026 11:32:02 +0100 Subject: [PATCH 09/11] check for null on queue add User may not always double check his param is found after param_list_find so add this as an added safety aginst users --- src/param/param_queue.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/param/param_queue.c b/src/param/param_queue.c index 8bed592d..6fdf968d 100644 --- a/src/param/param_queue.c +++ b/src/param/param_queue.c @@ -43,6 +43,10 @@ void param_queue_init(param_queue_t *queue, void *buffer, int buffer_size, int u int param_queue_add(param_queue_t *queue, const param_t *param, int offset, void *value) { + if (param == NULL) { + return -1; + } + /* Ensure we always send nodeid on the first element of the queue */ if (queue->used == 0) { queue->last_node = UINT16_MAX; From 7437cd7a54ba6fdafe175808de0dec6f74398bde Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Mon, 2 Feb 2026 11:33:23 +0100 Subject: [PATCH 10/11] const fix for param_list dynamic --- src/param/list/param_list.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/param/list/param_list.c b/src/param/list/param_list.c index c1a908c8..b7f4cb4b 100644 --- a/src/param/list/param_list.c +++ b/src/param/list/param_list.c @@ -148,11 +148,11 @@ int param_list_remove(int node, uint8_t verbose) { int count = 0; param_list_iterator i = {}; - param_t * iter_param = param_list_iterate(&i); + const param_t * iter_param = param_list_iterate(&i); while (iter_param) { - param_t * param = iter_param; // Free the current parameter after we have used it to iterate. + const param_t * param = iter_param; // Free the current parameter after we have used it to iterate. iter_param = param_list_iterate(&i); if (i.phase == 0) // Protection against removing static parameters @@ -175,7 +175,8 @@ int param_list_remove(int node, uint8_t verbose) { return count; } -void param_list_remove_specific(param_t * param, uint8_t verbose, int destroy) { + +void param_list_remove_specific(const param_t * param, uint8_t verbose, int destroy) { if (param_is_static(param)) { return; /* Nothing we can do :( */ @@ -445,9 +446,9 @@ static param_heap_t * param_list_alloc(int type, int array_size) { return param_heap; } -static void param_list_destroy_impl(param_t * param) { +static void param_list_destroy_impl(const param_t * param) { free(param->addr); - free(param); + free((void *) param); } #endif @@ -579,7 +580,7 @@ int param_list_download(int node, int timeout, int list_version, int include_rem return count; } -void param_list_destroy(param_t * param) { +void param_list_destroy(const param_t * param) { param_list_destroy_impl(param); } @@ -619,15 +620,15 @@ param_t * param_list_create_remote(int id, int node, int type, uint32_t mask, in param->array_size = array_size; param->array_step = param_typesize(type); - param->vmem->ack_with_pull = false; - param->vmem->driver = NULL; - param->vmem->name = "REMOTE"; - param->vmem->read = NULL; - param->vmem->size = array_size*param_typesize(type); - param->vmem->type = storage_type; - param->vmem->vaddr = (uint64_t)(uintptr_t)param_heap->buffer; - param->vmem->big_endian = false; - param->vmem->write = NULL; + param_heap->vmem.ack_with_pull = false; + param_heap->vmem.driver = NULL; + param_heap->vmem.name = "REMOTE"; + param_heap->vmem.read = NULL; + param_heap->vmem.size = array_size*param_typesize(type); + param_heap->vmem.type = storage_type; + param_heap->vmem.vaddr = (uint64_t)(uintptr_t)param_heap->buffer; + param_heap->vmem.big_endian = false; + param_heap->vmem.write = NULL; strlcpy(param->name, name, 36); if (unit) { From c2b631963a73bffb30e67803e89cc68dd0fc927e Mon Sep 17 00:00:00 2001 From: Johan de Claville Christiansen Date: Mon, 2 Feb 2026 12:34:32 +0100 Subject: [PATCH 11/11] revert builtin publish queues (moved to userspace) The init, configure and periodic calls, together with shall publish hook was just too much traffic in and out of the library. This is more efficient and easy to read code to perform all in userspace code. --- include/param/param_server.h | 39 ------------------------------------ meson.build | 1 - meson_options.txt | 1 - src/param/param_server.c | 20 ------------------ 4 files changed, 61 deletions(-) diff --git a/include/param/param_server.h b/include/param/param_server.h index e29bf4eb..de170958 100644 --- a/include/param/param_server.h +++ b/include/param/param_server.h @@ -74,42 +74,3 @@ typedef enum { * @param packet */ void param_serve(csp_packet_t * packet); - -#if PARAM_NUM_PUBLISHQUEUES > 4 -#error A maximum number of four param queues are supported -#endif - -#if PARAM_NUM_PUBLISHQUEUES > 0 -typedef struct param_publish_s { - param_t * param; - uint32_t queue; -} param_publish_t; - -typedef enum { - PARAM_PUBLISHQUEUE_0 = 0, -#if PARAM_NUM_PUBLISHQUEUES >= 2 - PARAM_PUBLISHQUEUE_1 = 1, -#endif -#if PARAM_NUM_PUBLISHQUEUES >= 3 - PARAM_PUBLISHQUEUE_2 = 2, -#endif -#if PARAM_NUM_PUBLISHQUEUES >= 4 - PARAM_PUBLISHQUEUE_3 = 3, -#endif -} param_publish_id_t; - -#define PARAM_ADD_PUBLISH(paramname, queueid) \ -param_publish_t __param_publish_##paramname##queueid = { \ - .param = ¶mname, \ - .queue = queueid, \ -}; \ -__attribute__((section("param_publish"))) \ -param_publish_t const * _param_publish_##paramname##queueid = & __param_publish_##paramname##queueid; - -typedef bool (*param_shall_publish_t)(uint8_t queue); - -void param_publish_periodic(void); -void param_publish_configure(param_publish_id_t queueid, uint16_t destination, uint16_t periodicity_ms, csp_prio_t csp_prio); -void param_publish_init(param_shall_publish_t criteria_cb); - -#endif diff --git a/meson.build b/meson.build index dcafbf82..ac0cbdac 100644 --- a/meson.build +++ b/meson.build @@ -7,7 +7,6 @@ conf.set('PARAM_HAVE_SYS_QUEUE', get_option('list_dynamic') or get_option('list_ conf.set('PARAM_LIST_DYNAMIC', get_option('list_dynamic')) conf.set('PARAM_LIST_POOL', get_option('list_pool')) conf.set('PARAM_HAVE_TIMESTAMP', get_option('have_timestamp')) -conf.set('PARAM_NUM_PUBLISHQUEUES', get_option('num_publishqueues')) # From now on, VMEM API is 64bits, breaking earlier ABI. # New user code can use the fact that this macro is defined (its value is not relevant, just the fact that it is defined) diff --git a/meson_options.txt b/meson_options.txt index a1c5765f..42e2cb86 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -5,5 +5,4 @@ option('have_timestamp', type: 'boolean', value: true, description: 'Use paramet option('list_dynamic', type: 'boolean', value: false, description: 'Compile support for dynamic param list (requres sys/queue.h) and dynamic memory allocation') option('list_pool', type: 'integer', value: 0, description: 'Compile support for pre-allocated param list (requres sys/queue.h)') option('have_float', type: 'boolean', value: true, description: 'Support float/double') -option('num_publishqueues', type: 'integer', value: 0, description: 'Number of param publish queues required') option('test', type: 'boolean', value: false, description: 'Build GoogleTest based tests (requires gtest)') diff --git a/src/param/param_server.c b/src/param/param_server.c index 9b16ba37..a975a3a1 100644 --- a/src/param/param_server.c +++ b/src/param/param_server.c @@ -26,7 +26,6 @@ struct param_serve_context { csp_packet_t * request; csp_packet_t * response; param_queue_t q_response; - csp_conn_t * publish_conn; }; static int __allocate(struct param_serve_context *ctx) { @@ -39,9 +38,6 @@ static int __allocate(struct param_serve_context *ctx) { static void __send(struct param_serve_context *ctx, int end) { - ctx->response->data[1] = (end) ? PARAM_FLAG_END : 0; - ctx->response->length = ctx->q_response.used + 2; - if (ctx->q_response.version == 1) { ctx->response->data[0] = PARAM_PULL_RESPONSE; } else { @@ -50,21 +46,6 @@ static void __send(struct param_serve_context *ctx, int end) { ctx->response->data[1] = (end) ? PARAM_FLAG_END : 0; ctx->response->length = ctx->q_response.used + 2; - if (ctx->publish_conn != NULL) { - ctx->response->data[1] |= PARAM_FLAG_NOACK; - - ctx->response->id.flags = CSP_O_CRC32; - ctx->response->id.src = 0; - - if (ctx->publish_conn == NULL) { - printf("param transaction failure\n"); - return; - } - - csp_send(ctx->publish_conn, ctx->response); - return; - } - csp_sendto_reply(ctx->request, ctx->response, CSP_O_SAME); } @@ -93,7 +74,6 @@ static void param_serve_pull_request(csp_packet_t * request, int all, int versio ctx.q_response.version = version; /* If packet->data[1] == 1 ack with pull response */ int ack_with_pull = request->data[1] == 1 ? 1 : 0; - ctx.publish_conn = NULL; if (__allocate(&ctx) < 0) { csp_buffer_free(request);