From d6d4d87e0bd90a0bc14fb76e5a2f680d616f766c Mon Sep 17 00:00:00 2001 From: laurent <laurent Thomas> Date: Thu, 18 Aug 2022 16:43:26 +0200 Subject: [PATCH] code review comments --- common/config/config_cmdline.c | 4 ++- common/config/config_load_configmodule.c | 11 ++++++-- common/config/config_userapi.c | 32 ++++++++++++---------- common/config/libconfig/config_libconfig.c | 4 ++- common/utils/LOG/log.c | 4 +-- common/utils/load_module_shlib.c | 4 +++ common/utils/load_module_shlib.h | 10 +++---- openair1/SIMULATION/NR_PHY/dlschsim.c | 4 --- openair2/LAYER2/NR_MAC_gNB/main.c | 3 +- 9 files changed, 44 insertions(+), 32 deletions(-) diff --git a/common/config/config_cmdline.c b/common/config/config_cmdline.c index 5ba6db85473..bd5f835a51a 100644 --- a/common/config/config_cmdline.c +++ b/common/config/config_cmdline.c @@ -54,7 +54,9 @@ int parse_stringlist(paramdef_t *cfgoptions, char *val) { free(tmpval); - AssertFatal(MAX_LIST_SIZE > numelt, ""); + AssertFatal(MAX_LIST_SIZE > numelt, + "This piece of code use fixed size arry of constant #define MAX_LIST_SIZE %d\n", + MAX_LIST_SIZE ); config_check_valptr(cfgoptions, sizeof(char*), numelt); cfgoptions->numelt=numelt; diff --git a/common/config/config_load_configmodule.c b/common/config/config_load_configmodule.c index 3fedf3f0f9a..0e3ffc1309c 100644 --- a/common/config/config_load_configmodule.c +++ b/common/config/config_load_configmodule.c @@ -339,16 +339,23 @@ void end_configmodule(void) { pthread_mutex_lock(&cfgptr->memBlocks_mutex); printf ("[CONFIG] free %u config value pointers\n",cfgptr->numptrs); - int n=0; for(int i=0; i<cfgptr->numptrs ; i++) { if (cfgptr->oneBlock[i].ptrs != NULL && cfgptr->oneBlock[i].ptrsAllocated== true && cfgptr->oneBlock[i].toFree) { free(cfgptr->oneBlock[i].ptrs); memset(&cfgptr->oneBlock[i], 0, sizeof(cfgptr->oneBlock[i])); } } - + cfgptr->numptrs=0; pthread_mutex_unlock(&cfgptr->memBlocks_mutex); + if ( cfgptr->cfgmode ) + free(cfgptr->cfgmode); + + if ( cfgptr->argv_info ) + free( cfgptr->argv_info ); + + free(cfgptr); + cfgptr=NULL; } } diff --git a/common/config/config_userapi.c b/common/config/config_userapi.c index cebc59a95d6..9725c15845f 100644 --- a/common/config/config_userapi.c +++ b/common/config/config_userapi.c @@ -54,7 +54,9 @@ configmodule_interface_t *config_get_if(void) { static int managed_ptr_sz(void* ptr) { configmodule_interface_t * cfg=config_get_if(); - AssertFatal(cfg->numptrs < CONFIG_MAX_ALLOCATEDPTRS,""); + AssertFatal(cfg->numptrs < CONFIG_MAX_ALLOCATEDPTRS, + "This code use fixed size array as #define CONFIG_MAX_ALLOCATEDPTRS %d\n", + CONFIG_MAX_ALLOCATEDPTRS); int i; pthread_mutex_lock(&cfg->memBlocks_mutex); int numptrs=cfg->numptrs; @@ -70,19 +72,17 @@ static int managed_ptr_sz(void* ptr) { void *config_allocate_new(int sz, bool autoFree) { void *ptr = calloc(sz,1); - if (ptr != NULL) { - configmodule_interface_t * cfg=config_get_if(); - // add the memory piece in the managed memory pieces list - pthread_mutex_lock(&cfg->memBlocks_mutex); - int newBlockIdx=cfg->numptrs++; - oneBlock_t* tmp=&cfg->oneBlock[newBlockIdx]; - tmp->ptrs = (char *)ptr; - tmp->ptrsAllocated = true; - tmp->sz=sz; - tmp->toFree=autoFree; - pthread_mutex_unlock(&cfg->memBlocks_mutex); - } else - AssertFatal(false, "calloc fails\n"); + AssertFatal(ptr, "calloc fails\n"); + configmodule_interface_t * cfg=config_get_if(); + // add the memory piece in the managed memory pieces list + pthread_mutex_lock(&cfg->memBlocks_mutex); + int newBlockIdx=cfg->numptrs++; + oneBlock_t* tmp=&cfg->oneBlock[newBlockIdx]; + tmp->ptrs = (char *)ptr; + tmp->ptrsAllocated = true; + tmp->sz=sz; + tmp->toFree=autoFree; + pthread_mutex_unlock(&cfg->memBlocks_mutex); return ptr; } @@ -124,7 +124,9 @@ void config_check_valptr(paramdef_t *cfgoptions, int elt_sz, int nb_elt) { // difficult datamodel cfgoptions->strptr = config_allocate_new(sizeof(*cfgoptions->strptr), toFree); } else if ( cfgoptions->type == TYPE_STRINGLIST) { - AssertFatal(nb_elt<MAX_LIST_SIZE, ""); + AssertFatal(nb_elt<MAX_LIST_SIZE, + "This piece of code use fixed size arry of constant #define MAX_LIST_SIZE %d\n", + MAX_LIST_SIZE ); cfgoptions->strlistptr= config_allocate_new(sizeof(char*)*MAX_LIST_SIZE, toFree); for (int i=0; i<MAX_LIST_SIZE; i++) cfgoptions->strlistptr[i]= config_allocate_new(DEFAULT_EXTRA_SZ, toFree); diff --git a/common/config/libconfig/config_libconfig.c b/common/config/libconfig/config_libconfig.c index 52faaaeb468..59e958470e6 100644 --- a/common/config/libconfig/config_libconfig.c +++ b/common/config/libconfig/config_libconfig.c @@ -57,7 +57,9 @@ int read_strlist(paramdef_t *cfgoptions,config_setting_t *setting, char *cfgpath numelt=config_setting_length(setting); config_check_valptr(cfgoptions, sizeof(char *), numelt); st=0; - AssertFatal(MAX_LIST_SIZE > numelt, ""); + AssertFatal(MAX_LIST_SIZE > numelt, + "This piece of code use fixed size arry of constant #define MAX_LIST_SIZE %d\n", + MAX_LIST_SIZE ); for (int i=0; i< numelt ; i++) { str=config_setting_get_string_elem(setting,i); diff --git a/common/utils/LOG/log.c b/common/utils/LOG/log.c index e10e2887372..9e8e17a3a4a 100644 --- a/common/utils/LOG/log.c +++ b/common/utils/LOG/log.c @@ -381,8 +381,8 @@ void log_getconfig(log_t *g_log) } int register_log_component(char *name, - char *fext, - int compidx) + char *fext, + int compidx) { int computed_compidx=compidx; diff --git a/common/utils/load_module_shlib.c b/common/utils/load_module_shlib.c index 1c9aa8cab9a..15b42577e6a 100644 --- a/common/utils/load_module_shlib.c +++ b/common/utils/load_module_shlib.c @@ -281,5 +281,9 @@ void loader_reset() shlib->numfunc = 0; shlib->len_funcarray = 0; } + if(loader_data.shlibpath){ + free(loader_data.shlibpath); + loader_data.shlibpath=NULL; + } free(loader_data.shlibs); } diff --git a/common/utils/load_module_shlib.h b/common/utils/load_module_shlib.h index 7c54403f4df..e804edbfe7a 100644 --- a/common/utils/load_module_shlib.h +++ b/common/utils/load_module_shlib.h @@ -51,11 +51,11 @@ typedef struct { }loader_shlibdesc_t; typedef struct { - char *mainexec_buildversion; - char *shlibpath; - uint32_t maxshlibs; - uint32_t numshlibs; - loader_shlibdesc_t *shlibs; + char *mainexec_buildversion; + char *shlibpath; + uint32_t maxshlibs; + uint32_t numshlibs; + loader_shlibdesc_t *shlibs; }loader_data_t; /* function type of functions which may be implemented by a module */ diff --git a/openair1/SIMULATION/NR_PHY/dlschsim.c b/openair1/SIMULATION/NR_PHY/dlschsim.c index 21dadc12914..72a4774f056 100644 --- a/openair1/SIMULATION/NR_PHY/dlschsim.c +++ b/openair1/SIMULATION/NR_PHY/dlschsim.c @@ -659,10 +659,6 @@ int main(int argc, char **argv) if (ouput_vcd) vcd_signal_dumper_close(); end_configmodule(); - if (load_configmodule(argc, argv, CONFIG_ENABLECMDLINEONLY) == 0) { - exit_fun("[NR_DLSCHSIM] Error, configuration module init 2 failed\n"); - } - logInit(); loader_reset(); logTerm(); diff --git a/openair2/LAYER2/NR_MAC_gNB/main.c b/openair2/LAYER2/NR_MAC_gNB/main.c index 9948df1effa..b272561a2d1 100644 --- a/openair2/LAYER2/NR_MAC_gNB/main.c +++ b/openair2/LAYER2/NR_MAC_gNB/main.c @@ -238,8 +238,7 @@ void mac_top_init_gNB(ngran_node_t node_type) RC.nrmac[i]->pre_processor_ul = nr_init_fr1_ulsch_preprocessor(i, 0); } if (!IS_SOFTMODEM_NOSTATS_BIT) - - threadCreate(&RC.nrmac[i]->stats_thread, nrmac_stats_thread, (void*)RC.nrmac[i], "MAC_STATS", -1, sched_get_priority_min(SCHED_OAI)+1 ); + threadCreate(&RC.nrmac[i]->stats_thread, nrmac_stats_thread, (void*)RC.nrmac[i], "MAC_STATS", -1, sched_get_priority_min(SCHED_OAI)+1 ); mac_rrc_init(RC.nrmac[i], node_type); }//END for (i = 0; i < RC.nb_nr_macrlc_inst; i++) -- GitLab