Hi Hans Ack for the series. Minor comment, perhaps compare_sanamet() can be moved to common as amfd already has a copy.
Thanks Gary On 14/6/17, 5:17 pm, "Hans Nordebäck" <[email protected]> wrote: Hi Praveen, thanks for reviewing, for info, the patch series also implies the following improvements: avnd_comp_clc_cmd_execute after refactoring: cyclomatic complexity reduced from 23 to 19 (-17%) # stmts reduced from 150 to 98 (-35%) # lines in function reduced from 259 to 184 (-29%) /Thanks Hans On 06/14/2017 08:52 AM, praveen malviya wrote: > Ack for the series. > > > Thanks > PRaveen > > On 13-Jun-17 4:54 PM, Hans Nordeback wrote: >> --- >> src/amf/amfnd/avnd_comp.h | 134 >> ++++++++++++++++++++++++++-------------------- >> src/amf/amfnd/avnd_err.h | 4 +- >> src/amf/amfnd/avnd_hc.h | 2 +- >> src/amf/amfnd/avnd_proc.h | 10 ++-- >> src/amf/amfnd/avnd_util.h | 2 +- >> src/amf/amfnd/clc.cc | 125 >> ++++-------------------------------------- >> src/amf/amfnd/comp.cc | 35 ++++++++++++ >> src/amf/amfnd/compdb.cc | 36 +++---------- >> src/amf/amfnd/proxydb.cc | 2 +- >> 9 files changed, 138 insertions(+), 212 deletions(-) >> >> diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h >> index 611e90e11..68de4cc8e 100644 >> --- a/src/amf/amfnd/avnd_comp.h >> +++ b/src/amf/amfnd/avnd_comp.h >> @@ -31,6 +31,8 @@ >> #define AMF_AMFND_AVND_COMP_H_ >> #include <bitset> >> +#include <string> >> +#include <map> >> struct avnd_cb_tag; >> struct avnd_su_si_rec; >> @@ -57,7 +59,7 @@ struct avnd_srm_req_tag; >> /* clc event handler declaration */ >> typedef uint32_t (*AVND_COMP_CLC_FSM_FN)(struct avnd_cb_tag *, >> - struct avnd_comp_tag *); >> + AVND_COMP *); >> /* clc fsm events */ >> typedef enum avnd_comp_clc_pres_fsm_ev { >> @@ -141,7 +143,7 @@ typedef struct avnd_cbk_tag { >> AVSV_AMF_CBK_INFO *cbk_info; /* callbk info */ >> /* link to other elements */ >> - struct avnd_comp_tag *comp; /* bk ptr to the comp */ >> + AVND_COMP *comp; /* bk ptr to the comp */ >> struct avnd_cbk_tag *next; >> std::string comp_name; /* For checkpointing */ >> } AVND_COMP_CBK; >> @@ -182,7 +184,7 @@ typedef struct avnd_comp_csi_rec { >> wrt prv ha state */ >> /* links to other entities */ >> - struct avnd_comp_tag *comp; /* bk ptr to the comp */ >> + AVND_COMP *comp; /* bk ptr to the comp */ >> struct avnd_su_si_rec *si; /* bk ptr to the si >> record */ >> std::string comp_name; /* For Checkpointing */ >> std::string si_name; /* For Checkpointing */ >> @@ -256,7 +258,7 @@ typedef struct avnd_hc_rec_tag { >> uint32_t opq_hdl; /* hdl returned by hdl-mngr (used during tmr >> expiry) */ >> AVND_COMP_HC_STATUS status; /* indicates status of hc rec */ >> - struct avnd_comp_tag *comp; /* back ptr to the comp */ >> + AVND_COMP *comp; /* back ptr to the comp */ >> struct avnd_hc_rec_tag *next; >> std::string comp_name; /* For checkpoiting */ >> } AVND_COMP_HC_REC; >> @@ -278,7 +280,7 @@ typedef struct avnd_pm_rec { >> } rec_rcvr; >> /* links to other entities */ >> - struct avnd_comp_tag *comp; /* back ptr to the comp */ >> + AVND_COMP *comp; /* back ptr to the comp */ >> } AVND_COMP_PM_REC; >> /*########################################################################## >> @@ -288,7 +290,7 @@ typedef struct avnd_pm_rec { >> /* proxied info */ >> typedef struct avnd_pxied_rec { >> NCS_DB_LINK_LIST_NODE comp_dll_node; /* node in the comp-pxied >> dll */ >> - struct avnd_comp_tag *pxied_comp; /* ptr to the proxied comp */ >> + AVND_COMP *pxied_comp; /* ptr to the proxied comp */ >> } AVND_COMP_PXIED_REC; >> #define AVND_COMP_TYPE_LOCAL_NODE 0x00000001 >> @@ -319,100 +321,115 @@ enum UsedComptypeAttrs { >> NumAttrs >> }; >> -typedef struct avnd_comp_tag { >> - NCS_DB_LINK_LIST_NODE su_dll_node; /* su dll node (key is >> inst-level) */ >> +class AVND_COMP { >> + public: >> + // TODO(uabhano) replace the NCS_DB_LINK_LIST_NODE with C++ STL. >> Now su_dll_node must be first in AVND_COMP >> + // as the macro m_AVND_COMP_SU_DLL_NODE_OFFSET depends on the >> offset. offsetof is to be avoided in classes. >> + NCS_DB_LINK_LIST_NODE su_dll_node {}; /* su dll node (key is >> inst-level) */ >> + AVND_COMP() {} >> + ~AVND_COMP() {} >> std::string name; /* comp name */ >> std::string saAmfCompType; >> - uint32_t numOfCompCmdEnv; /* number of comp command environment >> variables */ >> - SaStringT *saAmfCompCmdEnv; /* comp command environment variables */ >> - uint32_t inst_level; /* comp instantiation level */ >> - uint32_t comp_hdl; /* hdl returned by hdl-mngr */ >> + uint32_t inst_level {}; /* comp instantiation level */ >> + >> + uint32_t comp_hdl {}; /* hdl returned by hdl-mngr */ >> /* component attributes */ >> - uint32_t flag; /* comp attributes */ >> - bool is_restart_en; /* flag to indicate if comp-restart is allowed */ >> - SaAmfCompCapabilityModelT cap; /* comp capability model */ >> - bool is_am_en; >> - bool is_hc_cmd_configured; >> + uint32_t flag {}; /* comp attributes */ >> + bool is_restart_en {}; /* flag to indicate if comp-restart is >> allowed */ >> + SaAmfCompCapabilityModelT cap {}; /* comp capability model */ >> + bool is_am_en {}; >> + bool is_hc_cmd_configured {}; >> /* clc info */ >> - AVND_COMP_CLC_INFO clc_info; >> + AVND_COMP_CLC_INFO clc_info {}; >> /* Update received flag, which will normally be false and will be >> * true if updates are received from the AVD on fail-over.*/ >> - bool avd_updt_flag; >> + bool avd_updt_flag {}; >> /* component registration info */ >> - SaAmfHandleT reg_hdl; /* registered handle value */ >> - MDS_DEST reg_dest; /* mds dest of the registering prc */ >> + SaAmfHandleT reg_hdl {}; /* registered handle value */ >> + MDS_DEST reg_dest {}; /* mds dest of the registering prc */ >> /* component states */ >> - SaAmfOperationalStateT oper; /* operational state */ >> - SaAmfPresenceStateT pres; /* presence state */ >> - SaAmfProxyStatusT proxy_status; /* status for component with proxy */ >> + SaAmfOperationalStateT oper {}; /* operational state */ >> + SaAmfPresenceStateT pres {}; /* presence state */ >> + SaAmfProxyStatusT proxy_status {}; /* status for component with >> proxy */ >> /* >> * component request info (healthcheck, passive >> * monitoring, protection group tracking etc) >> */ >> - NCS_DB_LINK_LIST hc_list; /* health check info list */ >> - NCS_DB_LINK_LIST pm_list; /* passive monitoring req list */ >> + NCS_DB_LINK_LIST hc_list {}; /* health check info list */ >> + NCS_DB_LINK_LIST pm_list {}; /* passive monitoring req list */ >> - AVND_COMP_CBK *cbk_list; /* pending callback list */ >> + AVND_COMP_CBK *cbk_list {}; /* pending callback list */ >> /* call back responce timeout values */ >> - SaTimeT term_cbk_timeout; /* terminate */ >> - SaTimeT csi_set_cbk_timeout; /* csi set */ >> - SaTimeT quies_complete_cbk_timeout; /* quiescing_complete */ >> + SaTimeT term_cbk_timeout {}; /* terminate */ >> + SaTimeT csi_set_cbk_timeout {}; /* csi set */ >> + SaTimeT quies_complete_cbk_timeout {}; /* quiescing_complete */ >> /* Timer for qscing comlpete */ >> - AVND_TMR qscing_tmr; >> + AVND_TMR qscing_tmr {}; >> - SaTimeT csi_rmv_cbk_timeout; /* csi remove */ >> - SaTimeT pxied_inst_cbk_timeout; /* proxied instantiate */ >> - SaTimeT pxied_clean_cbk_timeout; /* proxied cleanup */ >> + SaTimeT csi_rmv_cbk_timeout {}; /* csi remove */ >> + SaTimeT pxied_inst_cbk_timeout {}; /* proxied instantiate */ >> + SaTimeT pxied_clean_cbk_timeout {}; /* proxied cleanup */ >> - AVND_CERR_INFO err_info; /* comp error information */ >> + AVND_CERR_INFO err_info {}; /* comp error information */ >> - uint32_t curr_proxied_cnt; /* proxied comp count (if any) >> + uint32_t curr_proxied_cnt {}; /* proxied comp count (if any) >> ## not used, to be deleted */ >> - NCS_DB_LINK_LIST csi_list; /* csi list */ >> + NCS_DB_LINK_LIST csi_list {}; /* csi list */ >> - struct avnd_su_tag *su; /* back ptr to parent SU */ >> + struct avnd_su_tag *su {}; /* back ptr to parent SU */ >> - struct avnd_comp_tag *pxy_comp; /* ptr to the proxy comp (if >> any) */ >> + AVND_COMP *pxy_comp {}; /* ptr to the proxy comp (if any) */ >> AVND_COMP_CLC_PRES_FSM_EV >> - pend_evt; /* stores last fsm event got in orph state */ >> + pend_evt {}; /* stores last fsm event got in orph state */ >> AVND_TMR >> - orph_tmr; /* proxied component registration timer alias >> orphaned timer */ >> + orph_tmr {}; /* proxied component registration timer alias >> orphaned timer */ >> - NCS_DB_LINK_LIST pxied_list; /* list of proxied comp in this >> proxy */ >> - NODE_ID node_id; /* It will used for internode proxy-proxied >> components. */ >> - uint32_t comp_type; /* Whether the component is LOCAL, INTERNODE >> or EXT */ >> - MDS_SYNC_SND_CTXT mds_ctxt; >> - bool reg_resp_pending; /* If the reg resp is pending from >> + NCS_DB_LINK_LIST pxied_list {}; /* list of proxied comp in this >> proxy */ >> + NODE_ID node_id {}; /* It will used for internode proxy-proxied >> components. */ >> + uint32_t comp_type {}; /* Whether the component is LOCAL, >> INTERNODE or EXT */ >> + MDS_SYNC_SND_CTXT mds_ctxt {}; >> + bool reg_resp_pending {}; /* If the reg resp is pending from >> proxied comp AvND, it >> true. */ >> std::string proxy_comp_name; /* Used for Checkpointing. */ >> - bool admin_oper; /*set to true if undergoing admin >> operation */ >> - int config_is_valid; /* Used to indicate that config has >> to be refreshed from >> + bool admin_oper {}; /*set to true if undergoing admin >> operation */ >> + int config_is_valid {}; /* Used to indicate that config >> has to be refreshed from >> IMM */ >> - bool assigned_flag; /* Used in finding multiple csi for a >> single comp while >> + bool assigned_flag {}; /* Used in finding multiple csi >> for a single comp while >> csi mod.*/ >> - bool pending_delete; /* Used in deleting component when su >> is in >> + bool pending_delete {}; /* Used in deleting component when >> su is in >> instantiated state.*/ >> - bool error_report_sent; /* true when error is repoted on >> component using >> + bool error_report_sent {}; /* true when error is repoted on >> component using >> saAmfComponentErrorReport() or >> saAmfComponentErrorReport_4()*/ >> - std::bitset<NumAttrs> *use_comptype_attr; >> + std::bitset<NumAttrs> *use_comptype_attr {}; >> SaInvocationT >> - term_cbq_inv_value; /* invocation value for termination >> callback. */ >> - SaVersionT version; // SAF version of comp. >> -} AVND_COMP; >> + term_cbq_inv_value {}; /* invocation value for termination >> callback. */ >> + SaVersionT version {}; // SAF version of comp. >> + >> + void add_cmd_env(const std::string &key_value); >> + void add_cmd_env(const std::string &name, const std::string &value); >> + void create_ncs_env_arg(NCS_OS_ENVIRON_ARGS* args); >> + >> + private: >> + std::map<std::string, std::string> env_var_ {}; >> + >> + // disallow copy and assign >> + AVND_COMP(const AVND_COMP&) = delete; >> + void operator=(const AVND_COMP&) = delete; >> +}; >> #define AVND_COMP_NULL ((AVND_COMP *)0) >> @@ -611,8 +628,9 @@ typedef struct avnd_comp_tag { >> #define m_AVND_COMP_IS_PROXIED(x) ((x)->proxy_comp) >> /* macro to retrieve component ptr from su dll node ptr */ >> -#define m_AVND_COMP_SU_DLL_NODE_OFFSET \ >> - ((uint8_t *)&(AVND_COMP_NULL->su_dll_node) - (uint8_t >> *)AVND_COMP_NULL) >> +/* TODO(uabhano) remove these macros */ >> +#define m_AVND_COMP_SU_DLL_NODE_OFFSET 0 >> + >> #define m_AVND_COMP_FROM_SU_DLL_NODE_GET(x) \ >> ((x) ? ((AVND_COMP *)(((uint8_t *)(x)) - >> m_AVND_COMP_SU_DLL_NODE_OFFSET)) : 0) >> @@ -851,7 +869,7 @@ extern uint32_t avnd_comp_cbk_send(struct >> avnd_cb_tag *, AVND_COMP *, >> AVSV_AMF_CBK_TYPE, >> AVND_COMP_HC_REC *, >> AVND_COMP_CSI_REC *); >> extern uint32_t avnd_comp_clc_cmd_execute(struct avnd_cb_tag *, >> - struct avnd_comp_tag *, >> + AVND_COMP *, >> enum >> avnd_comp_clc_cmd_type); >> extern AVND_COMP_HC_REC *avnd_comp_hc_get(AVND_COMP *, uint32_t, >> uint32_t); >> diff --git a/src/amf/amfnd/avnd_err.h b/src/amf/amfnd/avnd_err.h >> index de8c1172f..17e236d80 100644 >> --- a/src/amf/amfnd/avnd_err.h >> +++ b/src/amf/amfnd/avnd_err.h >> @@ -113,10 +113,10 @@ typedef struct avnd_err_tag { >> ***************************************************************************/ >> struct avnd_cb_tag; >> -struct avnd_comp_tag; >> +class AVND_COMP; >> struct avnd_su_tag; >> -extern uint32_t avnd_err_process(struct avnd_cb_tag *, struct >> avnd_comp_tag *, >> +extern uint32_t avnd_err_process(struct avnd_cb_tag *, AVND_COMP *, >> AVND_ERR_INFO *); >> extern uint32_t avnd_err_su_repair(struct avnd_cb_tag *, struct >> avnd_su_tag *); >> extern bool is_no_assignment_due_to_escalations(struct avnd_su_tag *); >> diff --git a/src/amf/amfnd/avnd_hc.h b/src/amf/amfnd/avnd_hc.h >> index 1f7ffcd34..cc51304b5 100644 >> --- a/src/amf/amfnd/avnd_hc.h >> +++ b/src/amf/amfnd/avnd_hc.h >> @@ -55,7 +55,7 @@ extern void avnd_hcdb_init(struct avnd_cb_tag *); >> extern AVND_HC *avnd_hcdb_rec_add(struct avnd_cb_tag *, >> AVND_HC_PARAM *, >> uint32_t *); >> extern uint32_t avnd_hcdb_rec_del(struct avnd_cb_tag *, >> AVSV_HLT_KEY *); >> -extern SaAisErrorT avnd_hc_config_get(struct avnd_comp_tag *comp); >> +extern SaAisErrorT avnd_hc_config_get(AVND_COMP *comp); >> extern SaAisErrorT avnd_hctype_config_get(SaImmHandleT immOmHandle, >> const std::string >> &comptype_dn); >> extern AVND_HCTYPE *avnd_hctypedb_rec_get(struct avnd_cb_tag *, >> diff --git a/src/amf/amfnd/avnd_proc.h b/src/amf/amfnd/avnd_proc.h >> index 8d5bea3c4..a65ae948f 100644 >> --- a/src/amf/amfnd/avnd_proc.h >> +++ b/src/amf/amfnd/avnd_proc.h >> @@ -36,7 +36,7 @@ >> struct avnd_cb_tag; >> struct avnd_evt_tag; >> -struct avnd_comp_tag; >> +class AVND_COMP; >> struct avnd_pxied_rec; >> typedef uint32_t (*AVND_EVT_HDLR)(struct avnd_cb_tag *, struct >> avnd_evt_tag *); >> @@ -46,11 +46,11 @@ void avnd_main_process(void); >> uint32_t avnd_evt_avd_node_up_evh(struct avnd_cb_tag *, struct >> avnd_evt_tag *); >> uint32_t avnd_evt_avd_reg_su_evh(struct avnd_cb_tag *, struct >> avnd_evt_tag *); >> uint32_t avnd_evt_avd_reg_comp_evh(struct avnd_cb_tag *, struct >> avnd_evt_tag *); >> -uint32_t avnd_comp_proxied_add(struct avnd_cb_tag *, struct >> avnd_comp_tag *, >> - struct avnd_comp_tag *, bool); >> +uint32_t avnd_comp_proxied_add(struct avnd_cb_tag *, AVND_COMP *, >> + AVND_COMP *, bool); >> ; >> -uint32_t avnd_comp_proxied_del(struct avnd_cb_tag *, struct >> avnd_comp_tag *, >> - struct avnd_comp_tag *, bool, >> +uint32_t avnd_comp_proxied_del(struct avnd_cb_tag *, AVND_COMP *, >> + AVND_COMP *, bool, >> struct avnd_pxied_rec *); >> uint32_t avnd_evt_avd_info_su_si_assign_evh(struct avnd_cb_tag *, >> struct avnd_evt_tag *); >> diff --git a/src/amf/amfnd/avnd_util.h b/src/amf/amfnd/avnd_util.h >> index 3add13aae..f32f448f8 100644 >> --- a/src/amf/amfnd/avnd_util.h >> +++ b/src/amf/amfnd/avnd_util.h >> @@ -31,7 +31,7 @@ >> #define AMF_AMFND_AVND_UTIL_H_ >> struct avnd_cb_tag; >> -struct avnd_comp_tag; >> +class AVND_COMP; >> enum avnd_comp_clc_cmd_type; >> extern const char *presence_state[]; >> diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc >> index c317f093b..13c9af614 100644 >> --- a/src/amf/amfnd/clc.cc >> +++ b/src/amf/amfnd/clc.cc >> @@ -2917,29 +2917,6 @@ uint32_t >> avnd_comp_clc_orph_restart_hdler(AVND_CB *cb, AVND_COMP *comp) { >> return rc; >> } >> -/** >> - * Determine if name is in the environment variable set >> - * >> - * @param name >> - * @param env_set >> - * @param env_counter >> - * >> - * @return bool >> - */ >> -static bool var_in_envset(const char *name, >> - const NCS_OS_ENVIRON_SET_NODE *env_set, >> - unsigned int env_counter) { >> - unsigned int i; >> - const char *var; >> - >> - for (i = 0, var = env_set[i].name; i < env_counter; >> - i++, var = env_set[i].name) { >> - if (strcmp(var, name) == 0) return true; >> - } >> - >> - return false; >> -} >> - >> /**************************************************************************** >> Name : avnd_comp_clc_cmd_execute >> @@ -2959,12 +2936,7 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB >> *cb, AVND_COMP *comp, >> AVND_COMP_CLC_CMD_TYPE cmd_type) { >> NCS_OS_PROC_EXECUTE_TIMED_INFO cmd_info; >> NCS_OS_ENVIRON_ARGS arg; >> - NCS_OS_ENVIRON_SET_NODE *env_set; >> - char env_val_nodeid[11]; >> - char env_val_comp_err[11]; /*we req only 10 */ >> - char env_var_name[] = "SA_AMF_COMPONENT_NAME"; >> - char env_var_nodeid[] = "NCS_ENV_NODE_ID"; >> - char env_var_comp_err[] = "OSAF_COMPONENT_ERROR_SOURCE"; >> + >> AVND_CLC_EVT *clc_evt; >> AVND_EVT *evt = 0; >> AVND_COMP_CLC_INFO *clc_info = &comp->clc_info; >> @@ -2972,10 +2944,6 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB >> *cb, AVND_COMP *comp, >> char *argv[AVND_COMP_CLC_PARAM_MAX + 2]; >> char tmp_argv[AVND_COMP_CLC_PARAM_MAX + >> 2][AVND_COMP_CLC_PARAM_SIZE_MAX]; >> uint32_t argc = 0, rc = NCSCC_RC_SUCCESS, count = 0; >> - unsigned int env_counter; >> - unsigned int i; >> - SaStringT env; >> - size_t env_set_nmemb; >> TRACE_ENTER2("'%s':CLC CLI command type:'%s'", comp->name.c_str(), >> clc_cmd_type[cmd_type]); >> @@ -3024,48 +2992,12 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB >> *cb, AVND_COMP *comp, >> } >> } >> + comp->add_cmd_env("NCS_ENV_NODE_ID", >> std::to_string(cb->node_info.nodeId)); >> + >> /* Allocate environment variable set */ >> - env_set_nmemb = comp->numOfCompCmdEnv + 3; >> - env_set = static_cast<NCS_OS_ENVIRON_SET_NODE *>( >> - calloc(env_set_nmemb, sizeof(NCS_OS_ENVIRON_SET_NODE))); >> memset(&cmd_info, 0, sizeof(NCS_OS_PROC_EXECUTE_TIMED_INFO)); >> memset(&arg, 0, sizeof(NCS_OS_ENVIRON_ARGS)); >> - /*** populate the env variable set ***/ >> - env_counter = 0; >> - >> - if (comp->saAmfCompCmdEnv != nullptr) { >> - while ((env = comp->saAmfCompCmdEnv[env_counter]) != nullptr) { >> - char *equalPos = strchr(env, '='); >> - if (equalPos == nullptr) { >> - LOG_ER("Unknown enviroment variable format '%s'. Should be >> 'var=value'", >> - env); >> - env_counter++; >> - continue; >> - } >> - env_set[env_counter].name = strndup(env, equalPos - env); >> - env_set[env_counter].value = strdup(equalPos + 1); >> - env_set[env_counter].overwrite = 1; >> - arg.num_args++; >> - env_counter++; >> - } >> - } >> - >> - /* comp name env */ >> - env_set[env_counter].overwrite = 1; >> - env_set[env_counter].name = strdup(env_var_name); >> - env_set[env_counter].value = strndup(comp->name.c_str(), >> comp->name.length()); >> - arg.num_args++; >> - env_counter++; >> - >> - /* node id env */ >> - env_set[env_counter].overwrite = 1; >> - env_set[env_counter].name = strdup(env_var_nodeid); >> - sprintf(env_val_nodeid, "%u", (uint32_t)(cb->node_info.nodeId)); >> - env_set[env_counter].value = strdup(env_val_nodeid); >> - arg.num_args++; >> - env_counter++; >> - >> /* Note:- we will set OSAF_COMPONENT_ERROR_SOURCE only for >> * cleanup script >> */ >> @@ -3073,13 +3005,7 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB >> *cb, AVND_COMP *comp, >> /* populate the env arg */ >> if (cmd_type == AVND_COMP_CLC_CMD_TYPE_CLEANUP) { >> /* error code, will be set only if we are cleaning up */ >> - memset(env_val_comp_err, '\0', sizeof(env_val_comp_err)); >> - env_set[env_counter].overwrite = 1; >> - env_set[env_counter].name = strdup(env_var_comp_err); >> - sprintf((char *)env_val_comp_err, "%u", >> (uint32_t)(comp->err_info.src)); >> - env_set[env_counter].value = strdup(env_val_comp_err); >> - arg.num_args++; >> - env_counter++; >> + comp->add_cmd_env("OSAF_COMPONENT_ERROR_SOURCE", >> std::to_string(comp->err_info.src)); >> } >> /* >> @@ -3102,46 +3028,17 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB >> *cb, AVND_COMP *comp, >> osafassert(csi); >> - /* allocate additional env_set memory for the CSI attributes */ >> - NCS_OS_ENVIRON_SET_NODE *tmp = >> static_cast<NCS_OS_ENVIRON_SET_NODE *>( >> - realloc(env_set, sizeof(NCS_OS_ENVIRON_SET_NODE) * >> - (env_set_nmemb + csi->attrs.number))); >> - osafassert(tmp); >> - env_set = tmp; >> - >> - /* initialize newly allocated memory */ >> - memset(&env_set[env_set_nmemb], 0, >> - sizeof(NCS_OS_ENVIRON_SET_NODE) * csi->attrs.number); >> - >> for (i = 0, csiattr = csi->attrs.list; i < csi->attrs.number; >> i++, csiattr++) { >> - if (var_in_envset(osaf_extended_name_borrow(&csiattr->name), >> env_set, >> - env_counter)) { >> - LOG_NO("Ignoring second (or more) value '%s' for '%s' CSI >> attr '%s'", >> - csiattr->string_ptr, comp->name.c_str(), >> - osaf_extended_name_borrow(&csiattr->name)); >> - continue; >> - } >> - >> TRACE("%s=%s", osaf_extended_name_borrow(&csiattr->name), >> csiattr->string_ptr); >> - env_set[env_counter].overwrite = 1; >> - env_set[env_counter].name = >> - strdup(osaf_extended_name_borrow(&csiattr->name)); >> - osafassert(env_set[env_counter].name != nullptr); >> - if (nullptr != csiattr->string_ptr) { >> - env_set[env_counter].value = strdup(csiattr->string_ptr); >> - osafassert(env_set[env_counter].value != nullptr); >> - } else { >> - env_set[env_counter].value = new char(); >> - } >> - arg.num_args++; >> - env_counter++; >> + >> + comp->add_cmd_env(osaf_extended_name_borrow(&csiattr->name), >> csiattr->string_ptr); >> } >> } /* if (comp->csi_list.n_nodes == 1) */ >> } >> - arg.env_arg = env_set; >> + comp->create_ncs_env_arg(&arg); >> /* tokenize the cmd */ >> m_AVND_COMP_CLC_STR_PARSE(clc_info->cmds[cmd_type - 1].cmd, scr, >> argc, argv, >> @@ -3173,11 +3070,11 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB >> *cb, AVND_COMP *comp, >> rc = ncs_os_process_execute_timed(&cmd_info); >> /* Remove the env_set structure */ >> - for (i = 0; i < env_counter; i++) { >> - free(env_set[i].name); >> - free(env_set[i].value); >> + for (unsigned int i = 0; i < arg.num_args; i++) { >> + free(arg.env_arg[i].name); >> + free(arg.env_arg[i].value); >> } >> - free(env_set); >> + delete [] arg.env_arg; >> if (NCSCC_RC_SUCCESS != rc) { >> TRACE_2("The CLC CLI command execution failed"); >> diff --git a/src/amf/amfnd/comp.cc b/src/amf/amfnd/comp.cc >> index 9dfe87a12..93aeddcf1 100644 >> --- a/src/amf/amfnd/comp.cc >> +++ b/src/amf/amfnd/comp.cc >> @@ -3086,3 +3086,38 @@ uint32_t avnd_amfa_mds_info_evh(AVND_CB *cb, >> AVND_EVT *evt) { >> TRACE_LEAVE(); >> return NCSCC_RC_SUCCESS; >> } >> + >> +void AVND_COMP::add_cmd_env(const std::string &key_value) { >> + std::string::size_type pos; >> + >> + pos = key_value.find('='); >> + >> + if (pos != std::string::npos) { >> + add_cmd_env(key_value.substr(0, pos - 1), key_value.substr(pos + >> 1)); >> + } else { >> + LOG_ER("Unknown environment variable format '%s'. Should be >> 'var=value'", >> + key_value.c_str()); >> + } >> +} >> + >> +void AVND_COMP::add_cmd_env(const std::string &key, const >> std::string &value) { >> + if (env_var_.insert({key, value}).second) { >> + TRACE("Environment variable %s added with value %s", >> key.c_str(), value.c_str()); >> + } else { >> + LOG_NO("Environment variable %s already exists with value %s", >> key.c_str(), value.c_str()); >> + } >> +} >> + >> +void AVND_COMP::create_ncs_env_arg(NCS_OS_ENVIRON_ARGS* args) { >> + args->num_args = env_var_.size(); >> + >> + NCS_OS_ENVIRON_SET_NODE *env = new >> NCS_OS_ENVIRON_SET_NODE[args->num_args]; >> + int i = 0; >> + for (const auto &it : env_var_) { >> + env[i].name = strdup(it.first.c_str()); >> + env[i].value = strdup(it.second.c_str()); >> + env[i].overwrite = 1; >> + i++; >> + } >> + args->env_arg = env; >> +} >> diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc >> index decf3abcb..0774e920b 100644 >> --- a/src/amf/amfnd/compdb.cc >> +++ b/src/amf/amfnd/compdb.cc >> @@ -1269,7 +1269,7 @@ static int comp_init(AVND_COMP *comp, const >> SaImmAttrValuesT_2 **attributes) { >> unsigned int num_of_ct_env = 0; >> unsigned int env_cntr = 0; >> const char *str; >> - SaStringT env; >> + >> SaImmHandleT immOmHandle; >> SaVersionT immVersion = {'A', 2, 15}; >> SaNameT node_name; >> @@ -1387,14 +1387,6 @@ static int comp_init(AVND_COMP *comp, const >> SaImmAttrValuesT_2 **attributes) { >> /* Set oper status to enable irrespective of comp category PI or >> NPI. */ >> m_AVND_COMP_OPER_STATE_SET(comp, SA_AMF_OPERATIONAL_ENABLED); >> - /* Remove any previous environment variables */ >> - if (comp->saAmfCompCmdEnv != nullptr) { >> - env_cntr = 0; >> - while ((env = comp->saAmfCompCmdEnv[env_cntr++]) != nullptr) >> delete env; >> - delete[] comp->saAmfCompCmdEnv; >> - comp->saAmfCompCmdEnv = nullptr; >> - } >> - >> /* Find out how many environment variables there are in our comp >> type */ >> num_of_ct_env = 0; >> if (comptype->saAmfCtDefCmdEnv != nullptr) { >> @@ -1406,29 +1398,21 @@ static int comp_init(AVND_COMP *comp, const >> SaImmAttrValuesT_2 **attributes) { >> immutil_getAttrValuesNumber(const_cast<SaImmAttrNameT>("saAmfCompCmdEnv"), >> attributes, &num_of_comp_env); >> - /* Store the total number of env variables */ >> - comp->numOfCompCmdEnv = num_of_ct_env + num_of_comp_env; >> - >> - /* Allocate total number of environment variables */ >> - comp->saAmfCompCmdEnv = new SaStringT[comp->numOfCompCmdEnv + 1](); >> - osafassert(comp->saAmfCompCmdEnv); >> - >> /* Copy environment variables from our comp type */ >> env_cntr = 0; >> while ((comptype->saAmfCtDefCmdEnv[env_cntr]) != nullptr) { >> - comp->saAmfCompCmdEnv[env_cntr] = >> - StrDup(comptype->saAmfCtDefCmdEnv[env_cntr]); >> - env_cntr++; >> + comp->add_cmd_env(comptype->saAmfCtDefCmdEnv[env_cntr++]); >> } >> /* Get environment variables from our IMM comp object */ >> for (i = 0; i < num_of_comp_env; i++, env_cntr++) { >> str = immutil_getStringAttr(attributes, "saAmfCompCmdEnv", i); >> osafassert(str); >> - comp->saAmfCompCmdEnv[env_cntr] = StrDup(str); >> - osafassert(comp->saAmfCompCmdEnv[env_cntr]); >> + comp->add_cmd_env(str); >> } >> + comp->add_cmd_env("SA_AMF_COMPONENT_NAME", comp->name); >> + >> /* The env string array will be terminated by zero due to the c++ >> * value-initialized new above */ >> @@ -1454,15 +1438,7 @@ done1: >> * @return >> */ >> void avnd_comp_delete(AVND_COMP *comp) { >> - SaStringT env; >> - >> /* Free saAmfCompCmdEnv[i] before freeing saAmfCompCmdEnv */ >> - if (comp->saAmfCompCmdEnv != nullptr) { >> - int env_counter = 0; >> - while ((env = comp->saAmfCompCmdEnv[env_counter++]) != nullptr) >> - delete[] env; >> - delete[] comp->saAmfCompCmdEnv; >> - } >> delete comp->use_comptype_attr; >> delete comp; >> @@ -1489,7 +1465,7 @@ static AVND_COMP *avnd_comp_create(const >> std::string &comp_name, >> TRACE_ENTER2("%s", comp_name.c_str()); >> - comp = new AVND_COMP(); >> + comp = new AVND_COMP; >> comp->use_comptype_attr = new std::bitset<NumAttrs>; >> comp->name = comp_name; >> diff --git a/src/amf/amfnd/proxydb.cc b/src/amf/amfnd/proxydb.cc >> index 4cb0acd93..69f2f4e52 100644 >> --- a/src/amf/amfnd/proxydb.cc >> +++ b/src/amf/amfnd/proxydb.cc >> @@ -171,7 +171,7 @@ AVND_COMP *avnd_internode_comp_add(AVND_CB *cb, >> const std::string &name, >> } >> /* a fresh comp... */ >> - comp = new AVND_COMP(); >> + comp = new AVND_COMP; >> comp->use_comptype_attr = new std::bitset<NumAttrs>; >> /* update the comp-name (patricia key) */ >> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
