You can subscribe to this list here.
2013 |
Jan
|
Feb
|
Mar
(2) |
Apr
(442) |
May
(1532) |
Jun
(148) |
Jul
(178) |
Aug
(165) |
Sep
(196) |
Oct
(265) |
Nov
(230) |
Dec
(312) |
---|---|---|---|---|---|---|---|---|---|---|---|---|
2014 |
Jan
(328) |
Feb
(254) |
Mar
(141) |
Apr
(378) |
May
(441) |
Jun
(374) |
Jul
(235) |
Aug
(349) |
Sep
(396) |
Oct
(238) |
Nov
(138) |
Dec
(109) |
2015 |
Jan
(86) |
Feb
(81) |
Mar
(250) |
Apr
(180) |
May
(159) |
Jun
(106) |
Jul
(211) |
Aug
(248) |
Sep
(516) |
Oct
(297) |
Nov
(194) |
Dec
(196) |
2016 |
Jan
(232) |
Feb
(328) |
Mar
(422) |
Apr
(244) |
May
(281) |
Jun
(210) |
Jul
(211) |
Aug
(563) |
Sep
(440) |
Oct
(317) |
Nov
(405) |
Dec
(224) |
2017 |
Jan
(207) |
Feb
(399) |
Mar
(373) |
Apr
(206) |
May
(213) |
Jun
(215) |
Jul
(81) |
Aug
(151) |
Sep
(126) |
Oct
(336) |
Nov
(179) |
Dec
(149) |
2018 |
Jan
(194) |
Feb
(118) |
Mar
(234) |
Apr
(190) |
May
(103) |
Jun
(100) |
Jul
(162) |
Aug
(139) |
Sep
(140) |
Oct
(140) |
Nov
(181) |
Dec
(54) |
2019 |
Jan
(94) |
Feb
(76) |
Mar
(49) |
Apr
(46) |
May
(52) |
Jun
(79) |
Jul
(43) |
Aug
(97) |
Sep
(88) |
Oct
(138) |
Nov
(104) |
Dec
(66) |
2020 |
Jan
(85) |
Feb
(107) |
Mar
(68) |
Apr
(61) |
May
(52) |
Jun
(23) |
Jul
(118) |
Aug
(67) |
Sep
(27) |
Oct
(33) |
Nov
(41) |
Dec
(24) |
2021 |
Jan
(19) |
Feb
(11) |
Mar
(47) |
Apr
(39) |
May
(51) |
Jun
(18) |
Jul
(19) |
Aug
(16) |
Sep
(35) |
Oct
(15) |
Nov
(10) |
Dec
(21) |
2022 |
Jan
(12) |
Feb
(11) |
Mar
(30) |
Apr
(6) |
May
(10) |
Jun
(12) |
Jul
(13) |
Aug
(10) |
Sep
(3) |
Oct
(4) |
Nov
(24) |
Dec
(25) |
2023 |
Jan
(3) |
Feb
(3) |
Mar
(11) |
Apr
(8) |
May
(2) |
Jun
|
Jul
(4) |
Aug
(5) |
Sep
(2) |
Oct
|
Nov
|
Dec
(2) |
2024 |
Jan
(9) |
Feb
(1) |
Mar
(2) |
Apr
(8) |
May
(8) |
Jun
(5) |
Jul
(6) |
Aug
(3) |
Sep
(3) |
Oct
(3) |
Nov
|
Dec
|
2025 |
Jan
|
Feb
|
Mar
(5) |
Apr
|
May
(4) |
Jun
(3) |
Jul
(4) |
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
From: Thien H. <thi...@en...> - 2025-07-17 01:40:50
|
Hi Tai, ACK from me without comment. Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Friday, July 11, 2025 3:02 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Thang Nguyen <tha...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] amf: Fix coding issue identified by codechecker [#3374] --- src/amf/amfd/comp.cc | 1 + src/amf/amfd/csiattr.cc | 1 - src/amf/amfd/sg_npm_fsm.cc | 7 +++++-- src/amf/amfnd/clc.cc | 1 + src/amf/amfnd/comp.cc | 2 +- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc index d5cdb9388..0577674c0 100644 --- a/src/amf/amfd/comp.cc +++ b/src/amf/amfd/comp.cc @@ -1717,6 +1717,7 @@ static SaAisErrorT ccb_completed_modify_hdlr(CcbUtilOperationData_t *opdata) { LOG_WA("Comp modify completed (STDBY): comp does not exist"); return SA_AIS_OK; } + osafassert(comp != nullptr); while ((attr_mod = opdata->param.modify.attrMods[i++]) != nullptr) { const SaImmAttrValuesT_2 *attribute = &attr_mod->modAttr; diff --git a/src/amf/amfd/csiattr.cc b/src/amf/amfd/csiattr.cc index f52722b39..91a8530c5 100644 --- a/src/amf/amfd/csiattr.cc +++ b/src/amf/amfd/csiattr.cc @@ -591,7 +591,6 @@ static void csiattr_modify_apply(CcbUtilOperationData_t *opdata) { memcpy(tmp_csi_attr->name_value.string_ptr, value, strlen(value) + 1); osaf_extended_name_alloc(tmp_csi_attr->name_value.string_ptr, &tmp_csi_attr->name_value.value); - i = 1; } else { for (i = 0; i < attribute->attrValuesNumber; i++) { char *value = *(char **)attribute->attrValues[i++]; diff --git a/src/amf/amfd/sg_npm_fsm.cc b/src/amf/amfd/sg_npm_fsm.cc index fad0a4369..c190d920c 100644 --- a/src/amf/amfd/sg_npm_fsm.cc +++ b/src/amf/amfd/sg_npm_fsm.cc @@ -3802,13 +3802,16 @@ void SG_NPM::node_fail_su_oper(AVD_CL_CB *cb, AVD_SU *su) { if (su == su->sg_of_su->max_assigned_su) { /* Max_assigned_su is assigned quiesced state as part of SI transfer and the node went down */ + TRACE("SI transfer failed %s, max_assigned_su '%s'" + "min_assigned_su '%s'", + su->sg_of_su->si_tobe_redistributed->name.c_str(), + su->sg_of_su->max_assigned_su->name.c_str(), + su->sg_of_su->min_assigned_su->name.c_str()); su->sg_of_su->max_assigned_su = nullptr; su->sg_of_su->min_assigned_su = nullptr; su->sg_of_su->si_tobe_redistributed = nullptr; m_AVSV_SEND_CKPT_UPDT_ASYNC_RMV(cb, su->sg_of_su, AVSV_CKPT_AVD_SI_TRANS); - TRACE("SI transfer failed for SI '%s' as max_assigned_su went down", - su->sg_of_su->si_tobe_redistributed->name.c_str()); } } diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc index 3a196211a..3f9f98cbe 100644 --- a/src/amf/amfnd/clc.cc +++ b/src/amf/amfnd/clc.cc @@ -3112,6 +3112,7 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, uint32_t argc = 0, rc = NCSCC_RC_SUCCESS, count = 0; m_AVND_COMP_CLC_COUNT_AGRC(scr, comp->clc_info.cmds[cmd_type - 1].len, argc); char *argv[argc + 2]; + memset(argv, '\0', argc + 2); unsigned int env_counter; unsigned int i; SaStringT env; diff --git a/src/amf/amfnd/comp.cc b/src/amf/amfnd/comp.cc index 8c0bf35ff..ad729eb67 100644 --- a/src/amf/amfnd/comp.cc +++ b/src/amf/amfnd/comp.cc @@ -2151,7 +2151,7 @@ uint32_t avnd_comp_cbk_send(AVND_CB *cb, AVND_COMP *comp, /* determine csi name */ if (SA_AMF_CSI_TARGET_ALL != csi_flag) - csi_name = csi_rec->name.c_str(); + csi_name = csi_rec->name; else csi_name = ""; -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-07-08 03:36:25
|
--- src/imm/agent/imma_oi_api.cc | 2 +- src/imm/agent/imma_om_api.cc | 2 +- src/imm/apitest/implementer/applier.c | 2 +- src/imm/apitest/management/populate.c | 2 +- .../test_saImmConfigSyncrTimeoutImma.c | 4 +- src/imm/immloadd/imm_pbe_load.cc | 2 +- src/imm/immnd/ImmModel.cc | 5 +- src/imm/immnd/immnd_evt.c | 38 +++++----- src/imm/immnd/immnd_main.c | 23 +++++-- src/imm/immnd/immnd_proc.c | 11 +-- src/imm/tools/imm_admin.c | 18 +++-- src/imm/tools/imm_cfg.c | 69 +++++++++++++++---- src/imm/tools/imm_list.c | 24 +++++-- 13 files changed, 137 insertions(+), 65 deletions(-) diff --git a/src/imm/agent/imma_oi_api.cc b/src/imm/agent/imma_oi_api.cc index a503e61a5..09b752066 100644 --- a/src/imm/agent/imma_oi_api.cc +++ b/src/imm/agent/imma_oi_api.cc @@ -2806,7 +2806,6 @@ mds_send_fail: } } -clm_left: skip_over_send: bad_sync: bad_handle1: @@ -2839,6 +2838,7 @@ free_obj: free(p); /*free-2 */ } +clm_left: bad_handle: if (locked) { m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE); diff --git a/src/imm/agent/imma_om_api.cc b/src/imm/agent/imma_om_api.cc index 30d562b2e..4013bf003 100644 --- a/src/imm/agent/imma_om_api.cc +++ b/src/imm/agent/imma_om_api.cc @@ -7732,7 +7732,7 @@ mds_send_fail: free(req->searchParam.choice.oneAttrParam.attrName.buf); /*free-2 */ req->searchParam.choice.oneAttrParam.attrName.buf = NULL; req->searchParam.choice.oneAttrParam.attrName.size = 0; - if (searchParam->searchOneAttr.attrValue) { /*free-3 */ + if (searchParam && searchParam->searchOneAttr.attrValue) { /*free-3 */ immsv_evt_free_att_val(&(req->searchParam.choice.oneAttrParam.attrValue), searchParam->searchOneAttr.attrValueType); } diff --git a/src/imm/apitest/implementer/applier.c b/src/imm/apitest/implementer/applier.c index 12cc755d6..566532b3c 100644 --- a/src/imm/apitest/implementer/applier.c +++ b/src/imm/apitest/implementer/applier.c @@ -213,7 +213,7 @@ static void saImmOiAdminOperationCallback( SaAisErrorT err = immutil_saImmOiAdminOperationResult(immOiHandle, invocation, SA_AIS_OK); if (err != SA_AIS_OK) { - fprintf(stderr, "Reply on admin operation failed, err:%u\n", + fprintf(stderr, "Reply on admin operation failed, err:%d\n", err); } } diff --git a/src/imm/apitest/management/populate.c b/src/imm/apitest/management/populate.c index 28fea28bf..c634a482c 100644 --- a/src/imm/apitest/management/populate.c +++ b/src/imm/apitest/management/populate.c @@ -172,7 +172,7 @@ static range_obj_t *gen_pop_tree(unsigned int level, range_obj_t *rootObj, if (reminder) { fprintf(stderr, - "error - Returning with nonzero reminder r:%u ix:%u\n", + "error - Returning with nonzero reminder r:%u ix:%d\n", reminder, ix); abort(); } diff --git a/src/imm/apitest/management/test_saImmConfigSyncrTimeoutImma.c b/src/imm/apitest/management/test_saImmConfigSyncrTimeoutImma.c index 039b7e279..0cf5f09e7 100644 --- a/src/imm/apitest/management/test_saImmConfigSyncrTimeoutImma.c +++ b/src/imm/apitest/management/test_saImmConfigSyncrTimeoutImma.c @@ -37,12 +37,12 @@ static int get_pid_immnd() if (fscanf(f, "%d", &pid) == 0) { fprintf(stderr, "Could not read PID from file %s\n", osafimmnd_pid_file); - return -1; + pid = -1; } if (fclose(f) != 0) { fprintf(stderr, "Could not close file\n"); - return -1; + pid = -1; } return pid; } diff --git a/src/imm/immloadd/imm_pbe_load.cc b/src/imm/immloadd/imm_pbe_load.cc index 3fd9fde5c..a5b0e6e73 100644 --- a/src/imm/immloadd/imm_pbe_load.cc +++ b/src/imm/immloadd/imm_pbe_load.cc @@ -254,7 +254,7 @@ bool loadClassFromPbe(void *pbeHandle, SaImmHandleT immHandle, SaImmAttrFlagsT attrFlags = 0LL; SaImmAttrValueT attDflt = NULL; char buf[32]; - snprintf(buf, 32, "Row(%u): <", ++r); + snprintf(buf, 32, "Row(%d): <", ++r); std::string rowStr(buf); AttrInfo *attr_info = NULL; char *val; diff --git a/src/imm/immnd/ImmModel.cc b/src/imm/immnd/ImmModel.cc index 0caf5f11e..d0add57d8 100644 --- a/src/imm/immnd/ImmModel.cc +++ b/src/imm/immnd/ImmModel.cc @@ -5402,7 +5402,7 @@ SaAisErrorT ImmModel::adminOwnerChange(const struct immsv_a2nd_admown_set* req, if (i2 != sCcbVector.end() && (*i2)->isActive()) { std::string oldOwner; objectInfo->getAdminOwnerName(&oldOwner); - if (!release && (adm->mAdminOwnerName == oldOwner)) { + if (!release && adm && adm->mAdminOwnerName == oldOwner) { TRACE("Idempotent adminOwner set for %s on %s", oldOwner.c_str(), objectName.c_str()); } else if (!release && oldOwner.empty() @@ -5447,7 +5447,8 @@ SaAisErrorT ImmModel::adminOwnerChange(const struct immsv_a2nd_admown_set* req, if (i2 != sCcbVector.end() && (*i2)->isActive()) { std::string oldOwner; subObj->getAdminOwnerName(&oldOwner); - if (!release && adm->mAdminOwnerName == oldOwner) { + if (!release && adm && + adm->mAdminOwnerName == oldOwner) { TRACE("Idempotent adminOwner set for %s on %s", oldOwner.c_str(), subObjName.c_str()); } else { diff --git a/src/imm/immnd/immnd_evt.c b/src/imm/immnd/immnd_evt.c index 99e9452ef..46cb85b31 100644 --- a/src/imm/immnd/immnd_evt.c +++ b/src/imm/immnd/immnd_evt.c @@ -4718,7 +4718,7 @@ static void immnd_evt_proc_ccb_compl_rsp(IMMND_CB *cb, IMMND_EVT *evt, send_evt.info.imma.info.ccbCompl.implId = 0; send_evt.info.imma.info.ccbCompl.invocation = applCtn; - int ix = 0; + unsigned int ix = 0; for (; ix < applArrSize && err == SA_AIS_OK; ++ix) { SaImmOiHandleT implHandle = @@ -4806,7 +4806,7 @@ static void immnd_evt_proc_ccb_compl_rsp(IMMND_CB *cb, IMMND_EVT *evt, } if (arrSize) { - int ix; + unsigned int ix; memset(&send_evt, '\0', sizeof(IMMSV_EVT)); send_evt.type = IMMSV_EVT_TYPE_IMMA; send_evt.info.imma.type = @@ -4873,7 +4873,7 @@ static void immnd_evt_proc_ccb_compl_rsp(IMMND_CB *cb, IMMND_EVT *evt, evt->info.ccbUpcallRsp.ccbId; send_evt.info.imma.info.ccbCompl.implId = 0; send_evt.info.imma.info.ccbCompl.invocation = 0; - int ix = 0; + unsigned int ix = 0; for (; ix < applArrSize && err == SA_AIS_OK; ++ix) { SaImmOiHandleT implHandle = @@ -5490,7 +5490,7 @@ static void immnd_evt_pbe_rt_obj_deletes_rsp(IMMND_CB *cb, IMMND_EVT *evt, noreply: if (spApplConn && (evt->info.ccbUpcallRsp.result == SA_AIS_OK)) { - int ix = 0; + unsigned int ix = 0; SaImmHandleT tmp_hdl = m_IMMSV_PACK_HANDLE(spApplConn, cb->node_id); /*Fetch client node for Special applier OI */ @@ -5531,7 +5531,7 @@ noreply: done: if (arrSize) { - int ix; + unsigned int ix; for (ix = 0; ix < arrSize; ++ix) { free(objNameArr[ix]); } @@ -7307,7 +7307,7 @@ static void immnd_evt_proc_object_create(IMMND_CB *cb, IMMND_EVT *evt, /* Re-use the adminOwner member of the ccbCreate message to hold the invocation id. In this case, 0 => no reply is expected. */ - int ix = 0; + unsigned int ix = 0; for (; ix < arrSize && err == SA_AIS_OK; ++ix) { implHandle = m_IMMSV_PACK_HANDLE( applConnArr[ix], cb->node_id); @@ -7615,7 +7615,7 @@ static void immnd_evt_proc_object_modify(IMMND_CB *cb, IMMND_EVT *evt, /* Re-use the adminOwner member of the ccbModify message to hold the invocation id. In this case, 0 => no reply is expected. */ - int ix = 0; + unsigned int ix = 0; for (; ix < arrSize && err == SA_AIS_OK; ++ix) { bool isSpecialApplier = false; send_evt.info.imma.info.objModify.attrMods = @@ -8168,7 +8168,7 @@ static void immnd_evt_ccb_abort(IMMND_CB *cb, SaUint32T ccbId, memset(&send_evt, '\0', sizeof(IMMSV_EVT)); send_evt.type = IMMSV_EVT_TYPE_IMMA; send_evt.info.imma.type = IMMA_EVT_ND2A_OI_CCB_ABORT_UC; - int ix = 0; + unsigned int ix = 0; for (; ix < arrSize; ++ix) { /*Look up the client node for the implementer, using * implConn */ @@ -8215,7 +8215,7 @@ static void immnd_evt_ccb_abort(IMMND_CB *cb, SaUint32T ccbId, send_evt.type = IMMSV_EVT_TYPE_IMMA; send_evt.info.imma.type = IMMA_EVT_ND2A_OI_CCB_ABORT_UC; send_evt.info.imma.info.ccbCompl.ccbId = ccbId; - int ix = 0; + unsigned int ix = 0; for (; ix < applArrSize; ++ix) { SaImmOiHandleT implHandle = m_IMMSV_PACK_HANDLE(applConnArr[ix], cb->node_id); @@ -8427,7 +8427,7 @@ static void immnd_evt_proc_object_delete(IMMND_CB *cb, IMMND_EVT *evt, : IMMA_EVT_ND2A_OI_OBJ_DELETE_UC; send_evt.info.imma.info.objDelete.ccbId = evt->info.objDelete.ccbId; - int ix = 0; + unsigned int ix = 0; for (; ix < arrSize && err == SA_AIS_OK; ++ix) { if (implConnArr[ix] == 0) { /* implConn zero => ony for PBE or @@ -8528,7 +8528,7 @@ static void immnd_evt_proc_object_delete(IMMND_CB *cb, IMMND_EVT *evt, /* Re-use the adminOwner member of the ccbDelete message to hold the invocation id. In this case, 0 => no reply is expected. */ - int ix = 0; + unsigned int ix = 0; for (; ix < arrSize && err == SA_AIS_OK; ++ix) { /* Iterate over deleted objects */ SaUint32T *applConnArr = NULL; @@ -8543,7 +8543,7 @@ static void immnd_evt_proc_object_delete(IMMND_CB *cb, IMMND_EVT *evt, cb, &objName, evt->info.objDelete.ccbId, &applConnArr, true); - int ix2 = 0; + unsigned int ix2 = 0; for (; ix2 < arrSize2 && err == SA_AIS_OK; ++ix2) { /* Iterate over applier connections for object */ @@ -8661,7 +8661,7 @@ static void immnd_evt_proc_object_delete(IMMND_CB *cb, IMMND_EVT *evt, } if (arrSize) { - int ix; + unsigned int ix; free(implConnArr); implConnArr = NULL; free(invocArr); @@ -8921,7 +8921,7 @@ static void immnd_evt_proc_rt_object_delete(IMMND_CB *cb, IMMND_EVT *evt, } if (spApplConn && (err == SA_AIS_OK) && !delayedReply) { - int ix = 0; + unsigned int ix = 0; /* Indicates object is marked with SA_IMM_ATTR_NOTIFY and special applier is present at this node and we dont need to wait for ack from PBE (non persistent RTO or PBE not @@ -8963,7 +8963,7 @@ static void immnd_evt_proc_rt_object_delete(IMMND_CB *cb, IMMND_EVT *evt, done: if (arrSize) { - int ix; + unsigned int ix; for (ix = 0; ix < arrSize; ++ix) { free(objNameArr[ix]); } @@ -9201,7 +9201,7 @@ static void immnd_evt_proc_ccb_apply(IMMND_CB *cb, IMMND_EVT *evt, send_evt.type = IMMSV_EVT_TYPE_IMMA; send_evt.info.imma.type = IMMA_EVT_ND2A_OI_CCB_COMPLETED_UC; - int ix = 0; + unsigned int ix = 0; for (; ix < arrSize && err == SA_AIS_OK; ++ix) { /*Look up the client node for the implementer, @@ -9431,7 +9431,7 @@ skip_send: send_evt.info.imma.info.ccbCompl.implId = 0; send_evt.info.imma.info.ccbCompl.invocation = applCtn; - int ix = 0; + unsigned int ix = 0; for (; ix < applArrSize && err == SA_AIS_OK; ++ix) { implHandle = m_IMMSV_PACK_HANDLE( @@ -9514,7 +9514,7 @@ skip_send: evt->info.ccbId; send_evt.info.imma.info.ccbCompl.implId = 0; send_evt.info.imma.info.ccbCompl.invocation = 0; - int ix = 0; + unsigned int ix = 0; for (; ix < applArrSize && err == SA_AIS_OK; ++ix) { implHandle = m_IMMSV_PACK_HANDLE( @@ -10214,7 +10214,7 @@ uint32_t immnd_evt_proc_pbe_prto_purge_mutations(IMMND_CB *cb, IMMND_EVT *evt, uint32_t rc = NCSCC_RC_SUCCESS; IMMSV_EVT send_evt; IMMND_IMM_CLIENT_NODE *cl_node = NULL; - int ix = 0; + unsigned int ix = 0; osafassert(reqConnArr); memset(&send_evt, '\0', sizeof(IMMSV_EVT)); send_evt.type = IMMSV_EVT_TYPE_IMMA; diff --git a/src/imm/immnd/immnd_main.c b/src/imm/immnd/immnd_main.c index 50bbb57ec..30d504b56 100644 --- a/src/imm/immnd/immnd_main.c +++ b/src/imm/immnd/immnd_main.c @@ -148,23 +148,36 @@ static char** parse_reserved_class_names(const char* input) LOG_ER("The reserved name `%s` is invalid!", token); goto freedata; } - result = (char**)realloc(result, sizeof(char*) * ++n_elements); + char **temp = + (char **)realloc(result, sizeof(char *) * ++n_elements); + if (temp == NULL) { + LOG_ER("Memory allocation failed during realloc!"); + goto freedata; + } + result = temp; result[n_elements - 1] = strdup(token); token = strsep(&dup, ","); } - result = (char**)realloc(result, sizeof(char*) * (n_elements + 1)); + char **temp = (char**)realloc(result, sizeof(char*) * (n_elements + 1)); + if (temp == NULL) { + LOG_ER("Final realloc failed!"); + goto freedata; + } + result = temp; result[n_elements] = 0; free(tofree); return result; freedata: - for (i = 0; i < n_elements; i++) { - if (result[i]) free(result[i]); + if (result != NULL) { + for (i = 0; i < n_elements; i++) { + if (result[i]) free(result[i]); + } + free(result); } free(tofree); - free(result); return NULL; } diff --git a/src/imm/immnd/immnd_proc.c b/src/imm/immnd/immnd_proc.c index 5df703799..3fa856d98 100644 --- a/src/imm/immnd/immnd_proc.c +++ b/src/imm/immnd/immnd_proc.c @@ -893,8 +893,9 @@ void immnd_adjustEpoch(IMMND_CB *cb, bool increment) */ } - int newEpoch = immModel_adjustEpoch(cb, cb->mMyEpoch, &continuationId, - &pbeConn, pbeNodeIdPtr, increment); + SaUint32T newEpoch = + immModel_adjustEpoch(cb, cb->mMyEpoch, &continuationId, &pbeConn, + pbeNodeIdPtr, increment); if (newEpoch != cb->mMyEpoch) { /*This case only relevant when persistent epoch overrides last epoch, i.e. after reload at cluster start. */ @@ -1424,8 +1425,8 @@ static void immnd_cleanTheHouse(IMMND_CB *cb, bool iAmCoordNow) &oi_cl_node); osafassert(oi_cl_node); osafassert(!(oi_cl_node->mIsStale)); - for (int ix = 0; ix < ccbIdArrSize; - ++ix) { + for (unsigned int ix = 0; + ix < ccbIdArrSize; ++ix) { TRACE_2( "Fetch ccb outcome for ccb%u, nodeId:%u, conn:%u implId:%u", ccbIdArr[ix], pbeNodeId, @@ -1723,7 +1724,7 @@ static bool immnd_ccbsTerminated(IMMND_CB *cb, SaUint32T duration, memset(&send_evt, '\0', sizeof(IMMSV_EVT)); send_evt.type = IMMSV_EVT_TYPE_IMMD; send_evt.info.immd.type = IMMD_EVT_ND2D_ABORT_CCB; - int ix; + unsigned int ix; immModel_getNonCriticalCcbs(cb, &ccbIdArr, &ccbIdArrSize); for (ix = 0; ix < ccbIdArrSize; ++ix) { diff --git a/src/imm/tools/imm_admin.c b/src/imm/tools/imm_admin.c index 1d908f53e..8e5c5d793 100644 --- a/src/imm/tools/imm_admin.c +++ b/src/imm/tools/imm_admin.c @@ -211,7 +211,7 @@ void print_param(SaImmAdminOperationParamsT_2 *param) ctime_r(&time, buf); buf[strlen(buf) - 1] = '\0'; /* Remove new line */ - printf("%-50s %-12s %llu (0x%llx, %s)\n", param->paramName, + printf("%-50s %-12s %lld (0x%llx, %s)\n", param->paramName, "SA_TIME_T", (*((SaTimeT *)param->paramBuffer)), (*((SaTimeT *)param->paramBuffer)), buf); } break; @@ -328,7 +328,7 @@ int main(int argc, char *argv[]) exit(EXIT_FAILURE); } break; - case 'O': + case 'O': { if (operationId != -1) { fprintf( stderr, @@ -337,9 +337,15 @@ int main(int argc, char *argv[]) } operationId = SA_IMM_PARAM_ADMOP_ID_ESC; params_len++; - params = realloc( + const SaImmAdminOperationParamsT_2 **tmp = realloc( params, (params_len + 1) * sizeof(SaImmAdminOperationParamsT_2 *)); + if (tmp == NULL){ + fprintf(stderr, "realloc() error"); + if (params) free(params); + exit(EXIT_FAILURE); + } + params = tmp; param = malloc(sizeof(SaImmAdminOperationParamsT_2)); params[params_len - 1] = param; params[params_len] = NULL; @@ -348,8 +354,8 @@ int main(int argc, char *argv[]) param->paramBuffer = malloc(sizeof(SaStringT)); *((SaStringT *)(param->paramBuffer)) = strdup(optarg); opName = strdup(optarg); - break; - case 'p': + } break; + case 'p': { params_len++; const SaImmAdminOperationParamsT_2 **tmp = realloc( params, (params_len + 1) * @@ -368,7 +374,7 @@ int main(int argc, char *argv[]) optarg); exit(EXIT_FAILURE); } - break; + } break; case 't': timeoutVal = strtoll(optarg, (char **)NULL, 10); diff --git a/src/imm/tools/imm_cfg.c b/src/imm/tools/imm_cfg.c index 24648e692..69fb371bc 100644 --- a/src/imm/tools/imm_cfg.c +++ b/src/imm/tools/imm_cfg.c @@ -241,7 +241,7 @@ static void free_attr_value(SaImmValueTypeT attrValueType, static void free_attr_values(SaImmAttrValuesT_2 *attrValues) { - int i; + SaUint32T i; if (attrValues) { free(attrValues->attrName); @@ -257,7 +257,7 @@ static void free_attr_values(SaImmAttrValuesT_2 *attrValues) static void free_attr_mod(SaImmAttrModificationT_2 *attrMod) { - int i; + SaUint32T i; if (attrMod) { if (attrMod->modAttr.attrName) free(attrMod->modAttr.attrName); @@ -594,10 +594,15 @@ int object_create(const SaNameT **objectNames, const SaImmClassNameT className, } } if (!attrAdded) { - - attrValues = realloc(attrValues, - (attr_len + 1) * - sizeof(SaImmAttrValuesT_2 *)); + SaImmAttrValuesT_2 **temp = NULL; + temp = realloc(attrValues, + (attr_len + 1) * + sizeof(SaImmAttrValuesT_2 *)); + if (temp == NULL) { + fprintf(stderr, "realloc() error"); + goto done; + } + attrValues = temp; attrValues[attr_len - 1] = attrValue; attrValues[attr_len] = NULL; attr_len++; @@ -845,9 +850,15 @@ int object_modify(const SaNameT **objectNames, char **optargs, int optargs_len) } if (!attrAdded) { - attrMods = realloc( - attrMods, (attr_len + 1) * - sizeof(SaImmAttrModificationT_2 *)); + SaImmAttrModificationT_2 **temp = NULL; + temp = realloc(attrMods, + (attr_len + 1) * + sizeof(SaImmAttrModificationT_2 *)); + if (temp == NULL) { + fprintf(stderr, "realloc() error"); + goto done; + } + attrMods = temp; attrMods[attr_len - 1] = attrMod; attrMods[attr_len] = NULL; attr_len++; @@ -1766,11 +1777,17 @@ static int imm_operation(int argc, char *argv[]) strictParse = 1; } break; - case 'a': - optargs = + case 'a': { + char **temp = NULL; + temp = realloc(optargs, ++optargs_len * sizeof(char *)); + if (temp == NULL) { + fprintf(stderr, "realloc() error"); + exit(EXIT_FAILURE); + } + optargs = temp; optargs[optargs_len - 1] = strdup(optarg); - break; + } break; case 'c': className = optarg; op = verify_setoption(op, CREATE_OBJECT); @@ -1933,10 +1950,23 @@ static int imm_operation(int argc, char *argv[]) } if (op == DELETE_CLASS) { + SaImmClassNameT *temp = NULL; while (optind < argc) { - classNames = + temp = realloc(classNames, (classNames_len + 1) * sizeof(SaImmClassNameT *)); + if (temp == NULL) { + fprintf(stderr, "operation DELETE_CLASS - " + "realloc failed\n"); + if (classNames) + free(classNames); + rc = EXIT_FAILURE; + if (!transaction_mode) { + goto done_om_finalize; + } + exit(rc); + } + classNames = temp; classNames[classNames_len - 1] = ((SaImmClassNameT)argv[optind++]); classNames[classNames_len++] = NULL; @@ -1979,10 +2009,19 @@ static int imm_operation(int argc, char *argv[]) goto done_om_finalize; } } else { + SaNameT **temp = NULL; while (optind < argc) { - objectNames = - realloc(objectNames, + temp = realloc(objectNames, (objectNames_len + 1) * sizeof(SaNameT *)); + if (temp == NULL) { + fprintf(stderr, "realloc failed\n"); + rc = EXIT_FAILURE; + if (!transaction_mode) { + goto done_om_finalize; + } + exit(rc); + } + objectNames = temp; objectName = objectNames[objectNames_len - 1] = malloc(sizeof(SaNameT)); objectNames[objectNames_len++] = NULL; diff --git a/src/imm/tools/imm_list.c b/src/imm/tools/imm_list.c index 680bac91f..58ead585c 100644 --- a/src/imm/tools/imm_list.c +++ b/src/imm/tools/imm_list.c @@ -103,7 +103,7 @@ static void print_attr_value_raw(SaImmValueTypeT attrValueType, printf("%llu", *((SaUint64T *)attrValue)); break; case SA_IMM_ATTR_SATIMET: - printf("%llu", *((SaTimeT *)attrValue)); + printf("%lld", *((SaTimeT *)attrValue)); break; case SA_IMM_ATTR_SAFLOATT: printf("%.8g", *((SaFloatT *)attrValue)); @@ -168,7 +168,7 @@ static void print_attr_value(SaImmValueTypeT attrValueType, ctime_r(&time, buf); buf[strlen(buf) - 1] = '\0'; /* Remove new line */ - printf("%llu (0x%llx, %s)", *((SaTimeT *)attrValue), + printf("%lld (0x%llx, %s)", *((SaTimeT *)attrValue), *((SaTimeT *)attrValue), buf); break; } @@ -412,7 +412,7 @@ static void display_object(const char *name, char delimiter, const SaImmAttrNameT *attributeNames) { - int i = 0, j; + SaUint32T i = 0, j; SaImmAttrValuesT_2 *attr; SaNameT objectName; SaAisErrorT error; @@ -509,15 +509,27 @@ int main(int argc, char *argv[]) break; switch (c) { - case 'a': - attributeNames = realloc( + case 'a': { + SaImmAttrNameT *temp = realloc( attributeNames, ++len * sizeof(SaImmAttrNameT)); + if (temp == NULL) { + fprintf(stderr, "realloc() error"); + if (attributeNames) { + for (int i = 0; i < len - 1; i++) { + if (attributeNames[i] != NULL) + free(attributeNames[i]); + } + free(attributeNames); + } + exit(EXIT_FAILURE); + } + attributeNames = temp; attributeNames[len - 2] = strdup(optarg); attributeNames[len - 1] = NULL; if (!is_existed_delimiter) delimiter = ':'; pretty_print = 0; - break; + } break; case 'c': class_desc_print = 1; break; -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-07-08 03:36:23
|
Summary: imm: Fix coding issues identified by Codechecker [#3376] Review request for Ticket(s): 3376 Peer Reviewer(s): Thang, Dat, Tai Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3376 Base revision: 6349a66032ea53f2b1757be2a80ed4e972f5733f Personal repository: git://git.code.sf.net/u/thienhuynh/review -------------------------------- Impacted area Impact y/n -------------------------------- Docs n Build system n RPM/packaging n Configuration files n Startup scripts n SAF services n OpenSAF services y Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): --------------------------------------------- *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision d04b619a01dfa8b1749e2463a72f34f24c7ecdd6 Author: thien.m.huynh <thi...@en...> Date: Tue, 8 Jul 2025 10:19:58 +0700 imm: Fix coding issues identified by Codechecker [#3376] Complete diffstat: ------------------ src/imm/agent/imma_oi_api.cc | 2 +- src/imm/agent/imma_om_api.cc | 2 +- src/imm/apitest/implementer/applier.c | 2 +- src/imm/apitest/management/populate.c | 2 +- .../management/test_saImmConfigSyncrTimeoutImma.c | 4 +- src/imm/immloadd/imm_pbe_load.cc | 2 +- src/imm/immnd/ImmModel.cc | 5 +- src/imm/immnd/immnd_evt.c | 38 ++++++------ src/imm/immnd/immnd_main.c | 23 ++++++-- src/imm/immnd/immnd_proc.c | 11 ++-- src/imm/tools/imm_admin.c | 18 ++++-- src/imm/tools/imm_cfg.c | 69 +++++++++++++++++----- src/imm/tools/imm_list.c | 24 ++++++-- 13 files changed, 137 insertions(+), 65 deletions(-) Testing Commands: ----------------- N/A Testing, Expected Results: -------------------------- N/A Conditions of Submission: ------------------------- ACK from reviewers Arch Built Started Linux distro ------------------------------------------- mips n n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: ------------------- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-07-03 03:35:02
|
Hi Thang, ACK without comment. Best Regards, Thien ________________________________ From: Thang Nguyen <tha...@en...> Sent: Thursday, May 15, 2025 8:30 AM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Tai Nguyen <tai...@en...> Cc: ope...@li... <ope...@li...>; Thang Nguyen <tha...@en...> Subject: [PATCH 1/1] pyosaf: support python 3.10+ [#3372] >From Python 3.10 and later, several abstract base classes (ABCs) like Iterable, Iterator, Mapping, Sequence, etc. were fully removed from the main collections module. These classes were moved to collections.abc in Python 3.3. Should adapt it to support on all python version. --- python/pyosaf/utils/immom/iterator.py | 5 ++++- python/pyosaf/utils/ntf/reader.py | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/python/pyosaf/utils/immom/iterator.py b/python/pyosaf/utils/immom/iterator.py index 7b0e0a8a4..0d545ab31 100644 --- a/python/pyosaf/utils/immom/iterator.py +++ b/python/pyosaf/utils/immom/iterator.py @@ -17,7 +17,10 @@ ############################################################################ """ IMM Search iterators """ from __future__ import print_function -from collections import Iterator +try: + from collections.abc import Iterator # Python 3.3+ +except ImportError: + from collections import Iterator # For older versions from ctypes import pointer, cast, c_void_p from pyosaf.saAis import eSaAisErrorT, SaNameT, unmarshalNullArray, SaStringT diff --git a/python/pyosaf/utils/ntf/reader.py b/python/pyosaf/utils/ntf/reader.py index b6334822a..7c94638ef 100644 --- a/python/pyosaf/utils/ntf/reader.py +++ b/python/pyosaf/utils/ntf/reader.py @@ -15,7 +15,10 @@ # ############################################################################ """ NTF reader utilities """ -from collections import Iterator +try: + from collections.abc import Iterator # Python 3.3+ +except ImportError: + from collections import Iterator # For older versions from pyosaf import saNtf from pyosaf.saAis import eSaAisErrorT from pyosaf.utils import log_err, bad_handle_retry -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-06-27 03:00:10
|
--- src/smf/agent/smfa_utils.c | 8 ++--- src/smf/smfd/SmfCampState.cc | 4 +-- src/smf/smfd/SmfCampaignWrapup.cc | 7 +++-- src/smf/smfd/SmfImmOperation.h | 2 +- src/smf/smfd/SmfUpgradeCampaign.cc | 12 ++++---- src/smf/smfd/SmfUpgradeProcedure.cc | 48 +++++++++++++++++++---------- 6 files changed, 49 insertions(+), 32 deletions(-) diff --git a/src/smf/agent/smfa_utils.c b/src/smf/agent/smfa_utils.c index d9c7cbe6c..550bedaaf 100644 --- a/src/smf/agent/smfa_utils.c +++ b/src/smf/agent/smfa_utils.c @@ -379,8 +379,8 @@ uint32_t smfa_cbk_ok_resp_process(SaSmfHandleT smfHandle, hdl_prev->next_hdl = hdl_list->next_hdl; /* Head node deleted.*/ - if ((hdl_list->hdl == - hdl_prev->hdl)) { + if (hdl_list->hdl == + hdl_prev->hdl) { cbk_list->hdl_list = hdl_prev->next_hdl; /* No more resp @@ -417,7 +417,7 @@ uint32_t smfa_cbk_ok_resp_process(SaSmfHandleT smfHandle, rmv_inv: cbk_prev->next_cbk = cbk_list->next_cbk; /* Last inv node.*/ - if ((cbk_list->inv_id == cbk_prev->inv_id)) { + if (cbk_list->inv_id == cbk_prev->inv_id) { cb->cbk_list = cbk_prev->next_cbk; } free(cbk_list); @@ -479,7 +479,7 @@ uint32_t smfa_cbk_err_resp_process(SaInvocationT invocation, SaSmfHandleT hdl) rmv_inv: cbk_prev->next_cbk = cbk_list->next_cbk; /* Last inv node.*/ - if ((cbk_list->inv_id == cbk_prev->inv_id)) { + if (cbk_list->inv_id == cbk_prev->inv_id) { cb->cbk_list = cbk_prev->next_cbk; } free(cbk_list); diff --git a/src/smf/smfd/SmfCampState.cc b/src/smf/smfd/SmfCampState.cc index 8f62fb0bc..0667bb1cd 100644 --- a/src/smf/smfd/SmfCampState.cc +++ b/src/smf/smfd/SmfCampState.cc @@ -375,7 +375,7 @@ SmfCampResultT SmfCampStateInitial::executeInit(SmfUpgradeCampaign *i_camp) { TRACE_ENTER(); TRACE("SmfCampStateExecuting::executeInit, Running campaign init actions"); - if (i_camp->m_campInit.execute() != SA_AIS_OK) { + if (!i_camp->m_campInit.execute()) { std::string error = "Campaign init failed"; LOG_ER("%s", error.c_str()); SmfCampaignThread::instance()->campaign()->setError(error); @@ -1847,7 +1847,7 @@ SmfCampResultT SmfCampRollingBack::rollbackInit(SmfUpgradeCampaign *i_camp) { // succeeds. Write the same value, just for synchronization purposes. changeState(i_camp, SmfCampRollingBack::instance()); - if (i_camp->m_campInit.rollback() != SA_AIS_OK) { + if (!i_camp->m_campInit.rollback()) { std::string error = "Campaign init rollback failed"; LOG_ER("%s", error.c_str()); SmfCampaignThread::instance()->campaign()->setError(error); diff --git a/src/smf/smfd/SmfCampaignWrapup.cc b/src/smf/smfd/SmfCampaignWrapup.cc index 8f5646424..52e887aeb 100644 --- a/src/smf/smfd/SmfCampaignWrapup.cc +++ b/src/smf/smfd/SmfCampaignWrapup.cc @@ -182,8 +182,11 @@ bool SmfCampaignWrapup::rollbackCampWrapup() { LOG_NO("CAMP: Campaign wrapup, rollback wrapup actions (%zu)", m_campWrapupAction.size()); for (auto& elem : m_campWrapupAction) { - SmfImmCcbAction* immCcb = NULL; - if ((immCcb = dynamic_cast<SmfImmCcbAction*>(elem)) != NULL) { + if (!elem) { + TRACE("SmfCampaignWrapup rollback campWrapupAction skip"); + continue; + } + if (dynamic_cast<SmfImmCcbAction*>(elem)) { /* Since noone of these IMM CCB has been executed it's no point in trying to roll them back */ TRACE("SmfCampaignWrapup skipping immCcb rollback %d", (*elem).getId()); diff --git a/src/smf/smfd/SmfImmOperation.h b/src/smf/smfd/SmfImmOperation.h index ae537b820..a7cf80129 100644 --- a/src/smf/smfd/SmfImmOperation.h +++ b/src/smf/smfd/SmfImmOperation.h @@ -135,7 +135,7 @@ class SmfImmOperation { // get the correct operation descriptor enum OperationType { NotSet, Create, Delete, Modify }; - const OperationType GetOperationType() { return imm_operation_; } + OperationType GetOperationType() const { return imm_operation_; } const modelmodify::CreateDescriptor GetCreateDescriptor() { return object_create_; } diff --git a/src/smf/smfd/SmfUpgradeCampaign.cc b/src/smf/smfd/SmfUpgradeCampaign.cc index 4a1591abb..ac0dd6bf2 100644 --- a/src/smf/smfd/SmfUpgradeCampaign.cc +++ b/src/smf/smfd/SmfUpgradeCampaign.cc @@ -447,7 +447,7 @@ SaAisErrorT SmfUpgradeCampaign::tooManyRestarts(bool *o_result) { TRACE_ENTER(); SaAisErrorT rc = SA_AIS_OK; SaImmAttrValuesT_2 **attributes; - int curCnt = 0; + SaUint32T curCnt = 0; /* Read the SmfCampRestartInfo object smfCampRestartCnt attr */ std::string obj = "smfRestartInfo=info," + @@ -472,8 +472,8 @@ SaAisErrorT SmfUpgradeCampaign::tooManyRestarts(bool *o_result) { SmfImmAttribute attrsmfCampRestartCnt; attrsmfCampRestartCnt.SetAttributeName("smfCampRestartCnt"); attrsmfCampRestartCnt.SetAttributeType("SA_IMM_ATTR_SAUINT32T"); - char buf[5]; - snprintf(buf, 4, "%u", curCnt); + char buf[11] = {}; + snprintf(buf, sizeof(buf), "%u", curCnt); attrsmfCampRestartCnt.AddAttributeValue(buf); imoCampRestartInfo.AddValue(attrsmfCampRestartCnt); @@ -484,8 +484,8 @@ SaAisErrorT SmfUpgradeCampaign::tooManyRestarts(bool *o_result) { } } - int maxCnt = smfd_cb->smfCampMaxRestart; - TRACE("maxCnt=%d, curCnt=%d", maxCnt, curCnt); + SaUint32T maxCnt = smfd_cb->smfCampMaxRestart; + TRACE("maxCnt=%u, curCnt=%u", maxCnt, curCnt); if (curCnt > maxCnt) { TRACE("TRUE"); *o_result = true; @@ -1035,7 +1035,7 @@ void SmfUpgradeCampaign::resetMaintenanceState() { uint32_t retry_cnt = 0; while (++retry_cnt <= MAX_NO_RETRIES) { rc = immUtil.doImmOperations(operations); - if (rc != SA_AIS_OK && rc == SA_AIS_ERR_TRY_AGAIN) { + if (rc == SA_AIS_ERR_TRY_AGAIN) { /* * TRY_AGAIN is returned only when ccb is aborted * with Resource abort in error string. diff --git a/src/smf/smfd/SmfUpgradeProcedure.cc b/src/smf/smfd/SmfUpgradeProcedure.cc index fae0af596..0787e928a 100644 --- a/src/smf/smfd/SmfUpgradeProcedure.cc +++ b/src/smf/smfd/SmfUpgradeProcedure.cc @@ -1268,9 +1268,13 @@ bool SmfUpgradeProcedure::mergeStepIntoSingleStep(SmfUpgradeProcedure *i_proc, LOG_NO("Copy the procedure init actions"); i_proc->addInitActions((*proc_elem).getInitActions()); for (const auto &actionElem : i_proc->getInitActions()) { + if (!actionElem) { + TRACE("Procedure init action %d skip", initActionId); + continue; + } const SmfCallbackAction *cbkAction = dynamic_cast<const SmfCallbackAction *>(actionElem); - if (cbkAction != NULL) { + if (cbkAction) { const_cast<SmfCallbackAction *>(cbkAction)->setCallbackProcedure(this); } // Renumber to action id aviod DN name collision in the merged procedure @@ -1282,9 +1286,13 @@ bool SmfUpgradeProcedure::mergeStepIntoSingleStep(SmfUpgradeProcedure *i_proc, LOG_NO("Copy the procedure wrapup actions"); i_proc->addWrapupActions((*proc_elem).getWrapupActions()); for (const auto &actionElem : i_proc->getWrapupActions()) { + if (!actionElem) { + TRACE("Procedure wrapup action %d skip", wrapupActionId); + continue; + } const SmfCallbackAction *cbkAction = dynamic_cast<const SmfCallbackAction *>(actionElem); - if (cbkAction != NULL) { + if (cbkAction) { const_cast<SmfCallbackAction *>(cbkAction)->setCallbackProcedure(this); } // Renumber to action id aviod DN name collision in the merged procedure @@ -2656,8 +2664,7 @@ SaAisErrorT SmfUpgradeProcedure::createImmStep(SmfUpgradeStep *i_step) { TRACE_ENTER(); std::string dnDeactUnit = "safSmfDu=smfDeactivationUnit"; std::string dnActUnit = "safSmfAu=smfActivationUnit"; - int strSize = 64; - char str[strSize]; + char str[11] = {}; /* Create the SaSmfStep object */ SmfImmRTCreateOperation icoSaSmfStep; @@ -2674,28 +2681,28 @@ SaAisErrorT SmfUpgradeProcedure::createImmStep(SmfUpgradeStep *i_step) { SmfImmAttribute attrsaSmfStepMaxRetry; attrsaSmfStepMaxRetry.SetAttributeName("saSmfStepMaxRetry"); attrsaSmfStepMaxRetry.SetAttributeType("SA_IMM_ATTR_SAUINT32T"); - snprintf(str, strSize, "%d", i_step->getMaxRetry()); + snprintf(str, sizeof(str), "%u", i_step->getMaxRetry()); attrsaSmfStepMaxRetry.AddAttributeValue(str); icoSaSmfStep.AddValue(attrsaSmfStepMaxRetry); SmfImmAttribute attrsaSmfStepRetryCount; attrsaSmfStepRetryCount.SetAttributeName("saSmfStepRetryCount"); attrsaSmfStepRetryCount.SetAttributeType("SA_IMM_ATTR_SAUINT32T"); - snprintf(str, strSize, "%d", i_step->getRetryCount()); + snprintf(str, sizeof(str), "%u", i_step->getRetryCount()); attrsaSmfStepRetryCount.AddAttributeValue(str); icoSaSmfStep.AddValue(attrsaSmfStepRetryCount); SmfImmAttribute attrsaSmfStepRestartOption; attrsaSmfStepRestartOption.SetAttributeName("saSmfStepRestartOption"); attrsaSmfStepRestartOption.SetAttributeType("SA_IMM_ATTR_SAUINT32T"); - snprintf(str, strSize, "%d", i_step->getRestartOption()); + snprintf(str, sizeof(str), "%u", i_step->getRestartOption()); attrsaSmfStepRestartOption.AddAttributeValue(str); icoSaSmfStep.AddValue(attrsaSmfStepRestartOption); SmfImmAttribute attrsaSmfStepState; attrsaSmfStepState.SetAttributeName("saSmfStepState"); attrsaSmfStepState.SetAttributeType("SA_IMM_ATTR_SAUINT32T"); - snprintf(str, strSize, "%d", i_step->getState()); + snprintf(str, sizeof(str), "%d", i_step->getState()); attrsaSmfStepState.AddAttributeValue(str); icoSaSmfStep.AddValue(attrsaSmfStepState); @@ -3350,9 +3357,6 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { std::list<std::string> stepList; TRACE_ENTER(); - SmfUpgradeStep *newStep = new (std::nothrow) SmfUpgradeStep; - osafassert(newStep != NULL); - // Read the single step from IMM if (immutil.getChildren(getDn(), stepList, SA_IMM_SUBLEVEL, "SaSmfStep") == false) { @@ -3375,6 +3379,9 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { return SA_AIS_ERR_NOT_EXIST; } + SmfUpgradeStep *newStep = new (std::nothrow) SmfUpgradeStep; + osafassert(newStep != NULL); + TRACE("Copy step basic data from IMM into the new merged step."); if (newStep->init((const SaImmAttrValuesT_2 **)attributes) != SA_AIS_OK) { LOG_NO( @@ -3409,7 +3416,7 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { if (newStep->getState() == SA_SMF_STEP_INITIAL) { mergeStepIntoSingleStep(this, newStep); // Just merge again, as before si-swap - addProcStep(newStep); + if (newStep) addProcStep(newStep); } else if (newStep->getState() == SA_SMF_STEP_EXECUTING) { // Fetch AU/DU and step swNode from IMM steps SaAisErrorT rc = readCampaignImmModel(newStep); @@ -3451,10 +3458,14 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { LOG_NO("Copy the procedure init actions"); addInitActions((*proc_iter).getInitActions()); for (const auto &actioniter : getInitActions()) { + if (!actioniter) { + TRACE("Procedure init action %d skip", initActionId); + continue; + } // For the callback actions, set new calback procedure pointer const SmfCallbackAction *cbkAction = dynamic_cast<const SmfCallbackAction *>(actioniter); - if (cbkAction != NULL) { + if (cbkAction) { const_cast<SmfCallbackAction *>(cbkAction)->setCallbackProcedure( this); } @@ -3469,11 +3480,15 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { LOG_NO("Copy the procedure wrapup actions"); addWrapupActions((*proc_iter).getWrapupActions()); for (const auto &actioniter : getWrapupActions()) { + if (!actioniter) { + TRACE("procedure wrapup action %d skip", wrapupActionId); + continue; + } // For the callback actions, set new calback procedure // For the callback actions, set new calback procedure pointer const SmfCallbackAction *cbkAction = dynamic_cast<const SmfCallbackAction *>(actioniter); - if (cbkAction != NULL) { + if (cbkAction) { const_cast<SmfCallbackAction *>(cbkAction)->setCallbackProcedure( this); } @@ -3745,7 +3760,6 @@ SaAisErrorT SmfUpgradeProcedure::bundleRefFromSsCampaignImmModel( SmfCampaignThread::instance()->campaign()->getUpgradeCampaign(); const std::vector<SmfUpgradeProcedure *> &procedures = camp->getOriginalProcedures(); - std::vector<SmfUpgradeProcedure *>::const_iterator proc_iter; std::list<SmfBundleRef> bundlesOldProcSS; std::list<SmfBundleRef *> bundlesOldProcRO; for (const auto &proc_elem : procedures) { @@ -4132,9 +4146,9 @@ bool SmfUpgradeProcedure::isCompRestartable(const std::string &i_compDN) { bool rc = true; SaImmAttrValuesT_2 **attributes; bool instanceCompDisableRestartIsSet = false; - SaBoolT instanceCompDisableRestart; + SaBoolT instanceCompDisableRestart = SA_FALSE; bool instanceCtDefDisableRestartIsSet = false; - SaBoolT instanceCtDefDisableRestart; + SaBoolT instanceCtDefDisableRestart = SA_FALSE; SmfImmUtils immUtil; -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-06-27 02:42:01
|
Summary: smf: fix coding issues identified by codechecker [#3369] Review request for Ticket(s): 3369 Peer Reviewer(s): Thang, Dat, Tai Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3369 Base revision: 6349a66032ea53f2b1757be2a80ed4e972f5733f Personal repository: git://git.code.sf.net/u/thienhuynh/review -------------------------------- Impacted area Impact y/n -------------------------------- Docs n Build system n RPM/packaging n Configuration files n Startup scripts n SAF services n OpenSAF services y Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): --------------------------------------------- *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision e831af5e523e6a961c2995dccb669ac51148bb52 Author: thien.m.huynh <thi...@en...> Date: Fri, 27 Jun 2025 09:11:30 +0700 smf: fix coding issues identified by codechecker [#3369] Complete diffstat: ------------------ src/smf/agent/smfa_utils.c | 8 +++---- src/smf/smfd/SmfCampState.cc | 4 ++-- src/smf/smfd/SmfCampaignWrapup.cc | 7 ++++-- src/smf/smfd/SmfImmOperation.h | 2 +- src/smf/smfd/SmfUpgradeCampaign.cc | 12 +++++----- src/smf/smfd/SmfUpgradeProcedure.cc | 48 ++++++++++++++++++++++++------------- 6 files changed, 49 insertions(+), 32 deletions(-) Testing Commands: ----------------- N/A Testing, Expected Results: -------------------------- N/A Conditions of Submission: ------------------------- ACK from reviewers Arch Built Started Linux distro ------------------------------------------- mips n n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: ------------------- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-06-05 09:07:14
|
Hi Thang, ACK with minor comments. Please check comment start with [Thien] Best Regards, Thien ________________________________ From: Thang Nguyen <tha...@en...> Sent: Monday, May 26, 2025 11:44 AM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Tai Nguyen <tai...@en...> Cc: ope...@li... <ope...@li...>; Thang Nguyen <tha...@en...> Subject: [PATCH 1/1] osaf: Add a timeout to etcdctl to avoid hanging [#3375] In etcd 3.4, it introduces automatic retries for failed RPCs. It can cause hang on a dead/unreachable etcd server, especially during txn, watch, or lock. The solution is to add a timeout shell command in etcdctl command. --- src/osaf/consensus/plugins/etcd3.plugin | 32 +++++++++++++------------ 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/osaf/consensus/plugins/etcd3.plugin b/src/osaf/consensus/plugins/etcd3.plugin index a5add53e7..9712fe3ed 100644 --- a/src/osaf/consensus/plugins/etcd3.plugin +++ b/src/osaf/consensus/plugins/etcd3.plugin @@ -24,8 +24,10 @@ readonly etcd_options="" readonly etcd_timeout="3s" readonly heartbeat_interval=2 readonly etcd_tolerance_timeout=6 +readonly command_timeout="3s" export ETCDCTL_API=3 +ETCDCTL="timeout $command_timeout etcdctl $etcd_options --dial-timeout=${etcd_timeout}" # get # retrieve <value> of <key> from key-value store @@ -38,7 +40,7 @@ export ETCDCTL_API=3 get() { readonly key="$1" - if output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout get "$directory$key" 2>&1) + if output=$($ETCDCTL get "$directory$key" 2>&1) then key_=$(echo "$output" | tail -n2 | head -n1) value=$(echo "$output" | tail -n1) @@ -76,7 +78,7 @@ setkey() { if [ $timeout -gt 0 ]; then # create lease - if output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout lease grant $timeout) + if output=$($ETCDCTL lease grant $timeout) then lease_id=$(echo $output | awk '{print $2}') lease_param="--lease="$lease_id"" @@ -87,7 +89,7 @@ setkey() { lease_param="" fi - if etcdctl $etcd_options --dial-timeout $etcd_timeout put "$directory$key" \ + if $ETCDCTL put "$directory$key" \ "$value" "$lease_param" >/dev/null then return 0 @@ -115,7 +117,7 @@ create_key() { if [ $timeout -gt 0 ]; then # create lease - if output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout lease grant $timeout) + if output=$($ETCDCTL lease grant $timeout) then lease_id=$(echo $output | awk '{print $2}') lease_param="--lease="$lease_id"" @@ -132,12 +134,12 @@ create_key() { put \""$directory$key"\" \""$value"\" "$lease_param" " - output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout txn <<< "$transaction") + output=$($ETCDCTL txn <<< "$transaction") if [[ "$output" == *"OK"* ]]; then return 0 fi - if output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout get "$directory$key" | tail -n1) + if output=$($ETCDCTL get "$directory$key" | tail -n1) then return 1 else @@ -165,7 +167,7 @@ setkey_match_prev() { if [ $timeout -gt 0 ]; then # create lease - if output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout lease grant $timeout) + if output=$($ETCDCTL lease grant $timeout) then lease_id=$(echo $output | awk '{print $2}') lease_param="--lease="$lease_id"" @@ -182,7 +184,7 @@ setkey_match_prev() { put \""$directory$key"\" \""$value"\" "$lease_param" " - output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout txn <<< "$transaction") + output=$($ETCDCTL txn <<< "$transaction") if [[ "$output" == *"OK"* ]]; then return 0 fi @@ -200,7 +202,7 @@ setkey_match_prev() { erase() { readonly key="$1" [Thien] carry into one line for better reading if $ETCDCTL del "$directory$key" >/dev/null 2>&1 - if etcdctl $etcd_options --dial-timeout $etcd_timeout \ + if $ETCDCTL \ del "$directory$key" >/dev/null 2>&1 then return 0 @@ -228,7 +230,7 @@ lock() { put \""$directory$keyname"\" \""$owner"\" " - output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout txn <<< "$transaction") + output=$($ETCDCTL txn <<< "$transaction") if [[ "$output" == *"OK"* ]]; then return 0 fi @@ -239,12 +241,12 @@ lock() { put \""$directory$keyname"\" \""$owner"\" " - output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout txn <<< "$transaction") + output=$($ETCDCTL txn <<< "$transaction") if [[ "$output" == *"OK"* ]]; then return 0 fi - current_owner=$(etcdctl $etcd_options --dial-timeout $etcd_timeout get "$directory$keyname" | tail -n1) + current_owner=$($ETCDCTL get "$directory$keyname" | tail -n1) # see if we already hold the lock if [ "$current_owner" = "$owner" ]; then return 0 @@ -295,7 +297,7 @@ unlock() { put \""$directory$keyname"\" \"\" " - output=$(etcdctl $etcd_options --dial-timeout $etcd_timeout txn <<< "$transaction") + output=$($ETCDCTL txn <<< "$transaction") if [[ "$output" == *"OK"* ]]; then return 0 fi @@ -315,7 +317,7 @@ unlock() { return 2 fi [Thien] carry into one line for better reading if $ETCDCTL del "$directory$keyname" >/dev/null 2>&1 - if etcdctl $etcd_options --dial-timeout $etcd_timeout \ + if $ETCDCTL \ del "$directory$keyname" >/dev/null 2>&1 then return 0 @@ -354,7 +356,7 @@ watch() { result=$? if [ "$result" -gt 1 ]; then # etcd down?, check the healthiness of self endpoint - $(etcdctl endpoint health >/dev/null 2>&1) + $($ETCDCTL endpoint health >/dev/null 2>&1) is_healthy=$? ((tol_timeout=tol_timeout+heartbeat_interval)) if [ $tol_timeout -ge $etcd_tolerance_timeout ] || [ $is_healthy -ne 0 ]; then -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-05-20 10:38:39
|
Summary: smf: fix coding issue identified by codechecker [#3369] Review request for Ticket(s): 3369 Peer Reviewer(s): Thang, Dat Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3369 Base revision: 0aea074508136aee5895c3f29d0b22c488c2a86f Personal repository: git://git.code.sf.net/u/thienhuynh/review -------------------------------- Impacted area Impact y/n -------------------------------- Docs n Build system n RPM/packaging n Configuration files n Startup scripts n SAF services n OpenSAF services y Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): --------------------------------------------- *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision b2fa5509487725b20033006e51c9424b8835eb38 Author: thien.m.huynh <thi...@en...> Date: Tue, 20 May 2025 15:46:18 +0700 smf: fix coding issue identified by codechecker [#3369] Complete diffstat: ------------------ src/smf/agent/smfa_utils.c | 8 ++++---- src/smf/smfd/SmfCampState.cc | 4 ++-- src/smf/smfd/SmfCampaignWrapup.cc | 4 ++-- src/smf/smfd/SmfImmOperation.h | 2 +- src/smf/smfd/SmfUpgradeCampaign.cc | 8 ++++---- src/smf/smfd/SmfUpgradeProcedure.cc | 22 ++++++++++++++-------- 6 files changed, 27 insertions(+), 21 deletions(-) Testing Commands: ----------------- N/A Testing, Expected Results: -------------------------- N/A Conditions of Submission: ------------------------- ACK from reviewers Arch Built Started Linux distro ------------------------------------------- mips n n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: ------------------- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-05-20 09:05:10
|
--- src/smf/agent/smfa_utils.c | 8 ++++---- src/smf/smfd/SmfCampState.cc | 4 ++-- src/smf/smfd/SmfCampaignWrapup.cc | 4 ++-- src/smf/smfd/SmfImmOperation.h | 2 +- src/smf/smfd/SmfUpgradeCampaign.cc | 8 ++++---- src/smf/smfd/SmfUpgradeProcedure.cc | 22 ++++++++++++++-------- 6 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/smf/agent/smfa_utils.c b/src/smf/agent/smfa_utils.c index d9c7cbe6c..550bedaaf 100644 --- a/src/smf/agent/smfa_utils.c +++ b/src/smf/agent/smfa_utils.c @@ -379,8 +379,8 @@ uint32_t smfa_cbk_ok_resp_process(SaSmfHandleT smfHandle, hdl_prev->next_hdl = hdl_list->next_hdl; /* Head node deleted.*/ - if ((hdl_list->hdl == - hdl_prev->hdl)) { + if (hdl_list->hdl == + hdl_prev->hdl) { cbk_list->hdl_list = hdl_prev->next_hdl; /* No more resp @@ -417,7 +417,7 @@ uint32_t smfa_cbk_ok_resp_process(SaSmfHandleT smfHandle, rmv_inv: cbk_prev->next_cbk = cbk_list->next_cbk; /* Last inv node.*/ - if ((cbk_list->inv_id == cbk_prev->inv_id)) { + if (cbk_list->inv_id == cbk_prev->inv_id) { cb->cbk_list = cbk_prev->next_cbk; } free(cbk_list); @@ -479,7 +479,7 @@ uint32_t smfa_cbk_err_resp_process(SaInvocationT invocation, SaSmfHandleT hdl) rmv_inv: cbk_prev->next_cbk = cbk_list->next_cbk; /* Last inv node.*/ - if ((cbk_list->inv_id == cbk_prev->inv_id)) { + if (cbk_list->inv_id == cbk_prev->inv_id) { cb->cbk_list = cbk_prev->next_cbk; } free(cbk_list); diff --git a/src/smf/smfd/SmfCampState.cc b/src/smf/smfd/SmfCampState.cc index 8f62fb0bc..c897f1ea8 100644 --- a/src/smf/smfd/SmfCampState.cc +++ b/src/smf/smfd/SmfCampState.cc @@ -375,7 +375,7 @@ SmfCampResultT SmfCampStateInitial::executeInit(SmfUpgradeCampaign *i_camp) { TRACE_ENTER(); TRACE("SmfCampStateExecuting::executeInit, Running campaign init actions"); - if (i_camp->m_campInit.execute() != SA_AIS_OK) { + if (i_camp->m_campInit.execute() != true) { std::string error = "Campaign init failed"; LOG_ER("%s", error.c_str()); SmfCampaignThread::instance()->campaign()->setError(error); @@ -1847,7 +1847,7 @@ SmfCampResultT SmfCampRollingBack::rollbackInit(SmfUpgradeCampaign *i_camp) { // succeeds. Write the same value, just for synchronization purposes. changeState(i_camp, SmfCampRollingBack::instance()); - if (i_camp->m_campInit.rollback() != SA_AIS_OK) { + if (i_camp->m_campInit.rollback() != true) { std::string error = "Campaign init rollback failed"; LOG_ER("%s", error.c_str()); SmfCampaignThread::instance()->campaign()->setError(error); diff --git a/src/smf/smfd/SmfCampaignWrapup.cc b/src/smf/smfd/SmfCampaignWrapup.cc index 8f5646424..018afe7a6 100644 --- a/src/smf/smfd/SmfCampaignWrapup.cc +++ b/src/smf/smfd/SmfCampaignWrapup.cc @@ -182,8 +182,8 @@ bool SmfCampaignWrapup::rollbackCampWrapup() { LOG_NO("CAMP: Campaign wrapup, rollback wrapup actions (%zu)", m_campWrapupAction.size()); for (auto& elem : m_campWrapupAction) { - SmfImmCcbAction* immCcb = NULL; - if ((immCcb = dynamic_cast<SmfImmCcbAction*>(elem)) != NULL) { + if (elem == NULL) continue; + if (dynamic_cast<SmfImmCcbAction*>(elem) != NULL) { /* Since noone of these IMM CCB has been executed it's no point in trying to roll them back */ TRACE("SmfCampaignWrapup skipping immCcb rollback %d", (*elem).getId()); diff --git a/src/smf/smfd/SmfImmOperation.h b/src/smf/smfd/SmfImmOperation.h index ae537b820..a7cf80129 100644 --- a/src/smf/smfd/SmfImmOperation.h +++ b/src/smf/smfd/SmfImmOperation.h @@ -135,7 +135,7 @@ class SmfImmOperation { // get the correct operation descriptor enum OperationType { NotSet, Create, Delete, Modify }; - const OperationType GetOperationType() { return imm_operation_; } + OperationType GetOperationType() const { return imm_operation_; } const modelmodify::CreateDescriptor GetCreateDescriptor() { return object_create_; } diff --git a/src/smf/smfd/SmfUpgradeCampaign.cc b/src/smf/smfd/SmfUpgradeCampaign.cc index 4a1591abb..bf8224384 100644 --- a/src/smf/smfd/SmfUpgradeCampaign.cc +++ b/src/smf/smfd/SmfUpgradeCampaign.cc @@ -447,7 +447,7 @@ SaAisErrorT SmfUpgradeCampaign::tooManyRestarts(bool *o_result) { TRACE_ENTER(); SaAisErrorT rc = SA_AIS_OK; SaImmAttrValuesT_2 **attributes; - int curCnt = 0; + SaUint32T curCnt = 0; /* Read the SmfCampRestartInfo object smfCampRestartCnt attr */ std::string obj = "smfRestartInfo=info," + @@ -484,8 +484,8 @@ SaAisErrorT SmfUpgradeCampaign::tooManyRestarts(bool *o_result) { } } - int maxCnt = smfd_cb->smfCampMaxRestart; - TRACE("maxCnt=%d, curCnt=%d", maxCnt, curCnt); + SaUint32T maxCnt = smfd_cb->smfCampMaxRestart; + TRACE("maxCnt=%u, curCnt=%u", maxCnt, curCnt); if (curCnt > maxCnt) { TRACE("TRUE"); *o_result = true; @@ -1035,7 +1035,7 @@ void SmfUpgradeCampaign::resetMaintenanceState() { uint32_t retry_cnt = 0; while (++retry_cnt <= MAX_NO_RETRIES) { rc = immUtil.doImmOperations(operations); - if (rc != SA_AIS_OK && rc == SA_AIS_ERR_TRY_AGAIN) { + if (rc == SA_AIS_ERR_TRY_AGAIN) { /* * TRY_AGAIN is returned only when ccb is aborted * with Resource abort in error string. diff --git a/src/smf/smfd/SmfUpgradeProcedure.cc b/src/smf/smfd/SmfUpgradeProcedure.cc index fae0af596..369dfa365 100644 --- a/src/smf/smfd/SmfUpgradeProcedure.cc +++ b/src/smf/smfd/SmfUpgradeProcedure.cc @@ -1250,6 +1250,7 @@ bool SmfUpgradeProcedure::mergeStepIntoSingleStep(SmfUpgradeProcedure *i_proc, LOG_NO( "SmfUpgradeProcedure::mergeStepIntoSingleStep: Procedure scope not found (forAddRemove/forModify)"); delete newStep; + newStep = nullptr; TRACE_LEAVE(); return false; } @@ -1268,6 +1269,7 @@ bool SmfUpgradeProcedure::mergeStepIntoSingleStep(SmfUpgradeProcedure *i_proc, LOG_NO("Copy the procedure init actions"); i_proc->addInitActions((*proc_elem).getInitActions()); for (const auto &actionElem : i_proc->getInitActions()) { + if (actionElem == NULL) continue; const SmfCallbackAction *cbkAction = dynamic_cast<const SmfCallbackAction *>(actionElem); if (cbkAction != NULL) { @@ -1282,6 +1284,7 @@ bool SmfUpgradeProcedure::mergeStepIntoSingleStep(SmfUpgradeProcedure *i_proc, LOG_NO("Copy the procedure wrapup actions"); i_proc->addWrapupActions((*proc_elem).getWrapupActions()); for (const auto &actionElem : i_proc->getWrapupActions()) { + if (actionElem == NULL) continue; const SmfCallbackAction *cbkAction = dynamic_cast<const SmfCallbackAction *>(actionElem); if (cbkAction != NULL) { @@ -2674,21 +2677,21 @@ SaAisErrorT SmfUpgradeProcedure::createImmStep(SmfUpgradeStep *i_step) { SmfImmAttribute attrsaSmfStepMaxRetry; attrsaSmfStepMaxRetry.SetAttributeName("saSmfStepMaxRetry"); attrsaSmfStepMaxRetry.SetAttributeType("SA_IMM_ATTR_SAUINT32T"); - snprintf(str, strSize, "%d", i_step->getMaxRetry()); + snprintf(str, strSize, "%u", i_step->getMaxRetry()); attrsaSmfStepMaxRetry.AddAttributeValue(str); icoSaSmfStep.AddValue(attrsaSmfStepMaxRetry); SmfImmAttribute attrsaSmfStepRetryCount; attrsaSmfStepRetryCount.SetAttributeName("saSmfStepRetryCount"); attrsaSmfStepRetryCount.SetAttributeType("SA_IMM_ATTR_SAUINT32T"); - snprintf(str, strSize, "%d", i_step->getRetryCount()); + snprintf(str, strSize, "%u", i_step->getRetryCount()); attrsaSmfStepRetryCount.AddAttributeValue(str); icoSaSmfStep.AddValue(attrsaSmfStepRetryCount); SmfImmAttribute attrsaSmfStepRestartOption; attrsaSmfStepRestartOption.SetAttributeName("saSmfStepRestartOption"); attrsaSmfStepRestartOption.SetAttributeType("SA_IMM_ATTR_SAUINT32T"); - snprintf(str, strSize, "%d", i_step->getRestartOption()); + snprintf(str, strSize, "%u", i_step->getRestartOption()); attrsaSmfStepRestartOption.AddAttributeValue(str); icoSaSmfStep.AddValue(attrsaSmfStepRestartOption); @@ -3359,6 +3362,7 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { LOG_NO( "SmfUpgradeProcedure::getImmStepsMergedSingleStep: Failed to get steps for procedure %s", getDn().c_str()); + delete newStep; TRACE_LEAVE(); return SA_AIS_ERR_NOT_EXIST; } @@ -3371,6 +3375,7 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { LOG_NO( "SmfUpgradeProcedure::getImmStepsMergedSingleStep: IMM data for step %s not found", (*stepit).c_str()); + delete newStep; TRACE_LEAVE(); return SA_AIS_ERR_NOT_EXIST; } @@ -3409,7 +3414,7 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { if (newStep->getState() == SA_SMF_STEP_INITIAL) { mergeStepIntoSingleStep(this, newStep); // Just merge again, as before si-swap - addProcStep(newStep); + if (newStep != nullptr) addProcStep(newStep); } else if (newStep->getState() == SA_SMF_STEP_EXECUTING) { // Fetch AU/DU and step swNode from IMM steps SaAisErrorT rc = readCampaignImmModel(newStep); @@ -3451,13 +3456,14 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { LOG_NO("Copy the procedure init actions"); addInitActions((*proc_iter).getInitActions()); for (const auto &actioniter : getInitActions()) { + if (actioniter == NULL) continue; // For the callback actions, set new calback procedure pointer const SmfCallbackAction *cbkAction = dynamic_cast<const SmfCallbackAction *>(actioniter); if (cbkAction != NULL) { const_cast<SmfCallbackAction *>(cbkAction)->setCallbackProcedure( this); - } + } // Renumber action id aviod DN name collision in the merged procedure // rollback data @@ -3469,6 +3475,7 @@ SaAisErrorT SmfUpgradeProcedure::getImmStepsMergedSingleStep() { LOG_NO("Copy the procedure wrapup actions"); addWrapupActions((*proc_iter).getWrapupActions()); for (const auto &actioniter : getWrapupActions()) { + if (actioniter == NULL) continue; // For the callback actions, set new calback procedure // For the callback actions, set new calback procedure pointer const SmfCallbackAction *cbkAction = @@ -3745,7 +3752,6 @@ SaAisErrorT SmfUpgradeProcedure::bundleRefFromSsCampaignImmModel( SmfCampaignThread::instance()->campaign()->getUpgradeCampaign(); const std::vector<SmfUpgradeProcedure *> &procedures = camp->getOriginalProcedures(); - std::vector<SmfUpgradeProcedure *>::const_iterator proc_iter; std::list<SmfBundleRef> bundlesOldProcSS; std::list<SmfBundleRef *> bundlesOldProcRO; for (const auto &proc_elem : procedures) { @@ -4132,9 +4138,9 @@ bool SmfUpgradeProcedure::isCompRestartable(const std::string &i_compDN) { bool rc = true; SaImmAttrValuesT_2 **attributes; bool instanceCompDisableRestartIsSet = false; - SaBoolT instanceCompDisableRestart; + SaBoolT instanceCompDisableRestart = SA_FALSE; bool instanceCtDefDisableRestartIsSet = false; - SaBoolT instanceCtDefDisableRestart; + SaBoolT instanceCtDefDisableRestart = SA_FALSE; SmfImmUtils immUtil; -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-05-08 04:13:08
|
Hi Thang, ACK without comment. Best Regards, Thien ________________________________ From: Thang Nguyen <tha...@en...> Sent: Monday, April 28, 2025 5:40 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Tai Nguyen <tai...@en...> Cc: ope...@li... <ope...@li...>; Thang Nguyen <tha...@en...> Subject: [PATCH 1/1] smfd: Correct exec level of balanced procedure [#3370] When the upgrade is configured as balanced in service upgrade (BISU), the exec level of balanced proc is set to one higher than the highest of the original proc used in balanced groups. The exec level of balanced procedures should not be set by another balanced procs. --- src/smf/smfd/SmfExecControl.cc | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/smf/smfd/SmfExecControl.cc b/src/smf/smfd/SmfExecControl.cc index 27584385f..5eeeb7cd9 100644 --- a/src/smf/smfd/SmfExecControl.cc +++ b/src/smf/smfd/SmfExecControl.cc @@ -140,6 +140,7 @@ bool setBalancedExecLevel(const std::vector<std::string>& nodesforss, for (auto node : nodes) { if (isNodeInGroup(node, nodesforss)) { ingroup = true; + break; } } if (ingroup) { @@ -156,19 +157,17 @@ bool setBalancedExecLevel(const std::vector<std::string>& nodesforss, } balanced_execlvl += 1; - // Move the exec-level forward for other procedures so we avoid to execute in - // parallel with balanced procedures - for (auto proc : ucamp->getProcedures()) { - if (proc->getExecLevel() >= balanced_execlvl) { - proc->setExecLevel(std::to_string(proc->getExecLevel() + numberofss)); - } - } - for (auto proc : ucamp->getProcedures()) { if (!proc->getBalancedGroup().empty()) { // This is a balanced procedure, set the new exec level proc->setExecLevel( std::to_string(proc->getExecLevel() + balanced_execlvl)); + } else { + // Move the exec-level forward for other procedures so + // we avoid to execute in parallel with balanced procedures + if (proc->getExecLevel() >= balanced_execlvl) { + proc->setExecLevel(std::to_string(proc->getExecLevel() + numberofss)); + } } } if (!merged.empty()) { -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-05-05 12:48:29
|
Hi Tai, ACK with minor comment on commit message. Short commit message should be start with verb like fix, correct, add,... That describes what the changes in the commit. E.g. Fix immxml-merge failure caused by encoding mismatch Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Monday, May 5, 2025 4:48 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Thang Nguyen <tha...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] imm: Failure when merging XML files with different encodings [#3371] The immxml-merge tool uses the encoding of the first input file to set the output file's encoding. If any subsequent input files require UTF-8 encoding but the first file is not encoded in UTF-8, then immxml-merge will fail. UTF-8 encoding will be applied to the output files if any input file uses UTF-8 encoding. --- src/imm/tools/immxml-merge | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/imm/tools/immxml-merge b/src/imm/tools/immxml-merge index 9a03d3a9b..6f1ae21d5 100755 --- a/src/imm/tools/immxml-merge +++ b/src/imm/tools/immxml-merge @@ -78,7 +78,7 @@ class MergedImmDocument(BaseImmDocument): self.classDict = {} self.regexpObj = None self.dn_regexpObj = None - self.firstSourceDoc = None + self.encoding = None self.objectList = [] self.objectDnNameDict = {} self.classes_parsed = 0 @@ -115,7 +115,6 @@ class MergedImmDocument(BaseImmDocument): abort_script("Did not find <imm:IMM-contents element in first " "source file") - self.firstSourceDoc = doc self.isResultDocInitialized = True trace("Done copying elements from first source document.") @@ -298,6 +297,9 @@ class MergedImmDocument(BaseImmDocument): doc = xml.dom.minidom.parse(file_name) # doc = self.open_xml_document_file_and_check_namespace(file_name) + if doc.encoding is not None and self.encoding is None: + if doc.encoding.lower() == "utf-8": + self.encoding = "utf-8" if not self.isResultDocInitialized: self.init_result_document(doc) @@ -362,17 +364,12 @@ class MergedImmDocument(BaseImmDocument): # I think there should not be imm validation functionality in merge # tool (use validate_immfile instead) - print_info_stderr("encoding in first source xml document: %s", - self.firstSourceDoc.encoding) tmp_output_file_name = Options.outputFilename + ".tmp" file_object = open(tmp_output_file_name, "w") - - if self.firstSourceDoc.encoding is not None \ - and self.firstSourceDoc.encoding.lower() == "utf-8": - encoding = "utf-8" + encoding = "utf-8" + if self.encoding == "utf-8": heading = "<?xml version=\"1.0\" encoding=\"utf-8\"?>" else: - encoding = None heading = "<?xml version=\"1.0\"?>" file_object.write(heading) -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-03-27 06:14:00
|
Hi Khuong, ACK from me. Best Regards, Thien ________________________________ From: Khuong Le <khu...@en...> Sent: Thursday, March 27, 2025 12:05 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Thang Nguyen <tha...@en...> Cc: ope...@li... <ope...@li...>; Khuong Le <khu...@en...> Subject: [PATCH 1/1] ntf: Add new configured variable for logger records timeout [#3363] Currently, the NTFS_LOGGER_RECORD_TIMEOUT default is set to 10s and cannot be changed. In some cases, notifications may become overdue and can be removed. We offer a way for this variable to be configured as an environment variable that can be exported from a configuration file, enabling flexible setting. We define limit for notification overdue timeout to range from 5s to 30s, with default still being 10s. --- src/ntf/README | 8 ++++++++ src/ntf/ntfd/NtfLogger.cc | 17 ++++++++++++++++- src/ntf/ntfd/NtfLogger.h | 1 + src/ntf/ntfd/NtfNotification.cc | 5 ++--- src/ntf/ntfd/NtfNotification.h | 2 +- src/ntf/ntfd/ntfd.conf | 6 ++++++ src/ntf/ntfd/ntfs.h | 4 +++- 7 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/ntf/README b/src/ntf/README index 5bf670647..155d00ba1 100644 --- a/src/ntf/README +++ b/src/ntf/README @@ -244,6 +244,14 @@ writing notification is fail for long time, NTF has to write a big number of notifications whenever handling sending notification request and that will delay to handle other requests come to NTFD. The value of variable is from 10 to 5000. +NTFSV_LOGGER_RECORD_TIMEOUT + +The logger record timeout is used to remove the overdue notification. +When the file system is unresponsive and the logger buffer stays full for a +long period of time, overload occurs. This variable is designed to fix the side +effect of this problem like: ntf reject incoming request due to overloaded. +The variable's value ranges from 5 to 30 seconds (by default, it is 10). + for debug see DEBUG. COMMAND LINE INTERFACE diff --git a/src/ntf/ntfd/NtfLogger.cc b/src/ntf/ntfd/NtfLogger.cc index 480c9e7b6..f6cc6de95 100644 --- a/src/ntf/ntfd/NtfLogger.cc +++ b/src/ntf/ntfd/NtfLogger.cc @@ -92,6 +92,20 @@ NtfLogger::NtfLogger() : readCounter(0), isLoggerBufferfull(false) { LOG_WA("Logger buffer is set too big, get default max instead: %d", logger_buffer_capacity); } + + /* Get the logger record timeout from the environment variable. + The value should be from 5s to 30s (Default is 10s). */ + logger_timeout_record = base::GetEnv("NTFSV_LOGGER_RECORD_TIMEOUT", + static_cast<uint32_t>(NTFSV_LOGGER_RECORD_TIMEOUT_DEFAULT)); + if (logger_timeout_record < NTFSV_LOGGER_RECORD_TIMEOUT_MIN) { + logger_timeout_record = NTFSV_LOGGER_RECORD_TIMEOUT_MIN; + LOG_WA("Logger timeout is set too small, get default min instead: %d", + NTFSV_LOGGER_RECORD_TIMEOUT_MIN); + } else if (logger_timeout_record > NTFSV_LOGGER_RECORD_TIMEOUT_MAX) { + logger_timeout_record = NTFSV_LOGGER_RECORD_TIMEOUT_MAX; + LOG_WA("Logger timeout is set too big, get default max instead: %d", + NTFSV_LOGGER_RECORD_TIMEOUT_MAX); + } } /* Callbacks */ @@ -392,7 +406,8 @@ bool NtfLogger::isExistNotification(SaInvocationT invocation) { void NtfLogger::logQueuedNotification() { if (!isLoggerBufferEmpty()) { NtfSmartPtr notification = queuedNotificationList.front(); - if (notification->is_overdue() && notification->isWaitingAck()) { + if (notification->is_overdue(logger_timeout_record) + && notification->isWaitingAck()) { LOG_NO("Notification overdue, remove notification Id: %llu", notification->getNotificationId()); dequeueNotification(); diff --git a/src/ntf/ntfd/NtfLogger.h b/src/ntf/ntfd/NtfLogger.h index 4af8fc3d8..4a3f81df7 100644 --- a/src/ntf/ntfd/NtfLogger.h +++ b/src/ntf/ntfd/NtfLogger.h @@ -83,6 +83,7 @@ class NtfLogger { typedef std::list<NtfSmartPtr> QueuedNotificationsList; QueuedNotificationsList queuedNotificationList; + uint32_t logger_timeout_record; uint32_t logger_buffer_capacity; // The flag if logger buffer is full. This is set when checking the logger diff --git a/src/ntf/ntfd/NtfNotification.cc b/src/ntf/ntfd/NtfNotification.cc index 957206dfb..213ad1b97 100644 --- a/src/ntf/ntfd/NtfNotification.cc +++ b/src/ntf/ntfd/NtfNotification.cc @@ -267,11 +267,10 @@ void NtfNotification::removeSubscription(unsigned int clientId, */ ntfsv_send_not_req_t* NtfNotification::getNotInfo() { return sendNotInfo_; } -bool NtfNotification::is_overdue() const { - uint32_t max_time = NTFSV_LOGGER_RECORD_TIMEOUT_S; +bool NtfNotification::is_overdue(uint32_t timeout) const { timespec queue_at = base::NanosToTimespec(queue_at_); timespec current = base::ReadMonotonicClock(); - timespec max_queue_time{static_cast<time_t>(max_time), 0}; + timespec max_queue_time{static_cast<time_t>(timeout), 0}; return (current - queue_at > max_queue_time); } diff --git a/src/ntf/ntfd/NtfNotification.h b/src/ntf/ntfd/NtfNotification.h index d025648cf..ec12975b9 100644 --- a/src/ntf/ntfd/NtfNotification.h +++ b/src/ntf/ntfd/NtfNotification.h @@ -68,7 +68,7 @@ class NtfNotification { SaNtfNotificationHeaderT* header(); ntfsv_send_not_req_t* sendNotInfo_; bool loggFromCallback_; - bool is_overdue() const; + bool is_overdue(uint32_t timeout) const; private: NtfNotification(); diff --git a/src/ntf/ntfd/ntfd.conf b/src/ntf/ntfd/ntfd.conf index f2f67496f..8df0fbf13 100644 --- a/src/ntf/ntfd/ntfd.conf +++ b/src/ntf/ntfd/ntfd.conf @@ -51,3 +51,9 @@ export NTFSV_ENV_HEALTHCHECK_KEY="Default" # notifications whenever handling sending notification request and that will delay # to handle other requests come to NTFD. The value of variable is from 10 to 5000. #export NTFSV_LOGGER_BUFFER_CAPACITY=10 + +# The logger record timeout is used to remove the overdue notification. +# The default timeout is 10s, uncomment the next line to configure the logger recored timeout. +# The range can be configure from 5s to 30s. +# export NTFSV_LOGGER_RECORD_TIMEOUT=10 + diff --git a/src/ntf/ntfd/ntfs.h b/src/ntf/ntfd/ntfs.h index ee0276211..56d30dba0 100644 --- a/src/ntf/ntfd/ntfs.h +++ b/src/ntf/ntfd/ntfs.h @@ -58,7 +58,9 @@ // Periodic timer. Using in the main poll when queue available #define NTFSV_LOGGER_PERODIC_POLL_TIMEOUT_MS 5000 // Allowed queue time before notification overdue -#define NTFSV_LOGGER_RECORD_TIMEOUT_S 10 +#define NTFSV_LOGGER_RECORD_TIMEOUT_DEFAULT 10 +#define NTFSV_LOGGER_RECORD_TIMEOUT_MIN 5 +#define NTFSV_LOGGER_RECORD_TIMEOUT_MAX 30 /* ======================================================================== * TYPE DEFINITIONS -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-03-27 03:33:56
|
Hi Khuong, Please check my comments from text highlight and start with [Thien]. Best Regards, Thien ________________________________ From: Khuong Le <khu...@en...> Sent: Wednesday, March 5, 2025 1:39 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Thang Nguyen <tha...@en...>; Tai Nguyen <tai...@en...>; Vu Luu <vu...@en...> Cc: ope...@li... <ope...@li...>; Khuong Le <khu...@en...> Subject: [PATCH 1/1] ntf: Add new configured variable for logger records timeout [#3363] Currently, the NTFS_LOGGER_RECORD_TIMEOUT default is set to 10s and cannot be changed. In some cases cases, notifications my may become overdue and can be removed. [Thien]: I suggest statement below: We offer a way for this variable to be configured as an environment variable that can be exported from a configuration file, enabling flexible setting. To allow flexible configuration of this variables, we provide a solution where it van be set as an environment variable, exportable from config file. We define limit for notification overdue timeout to range from 5s to 30s, with default still being 10s. --- src/ntf/README | 9 +++++++++ src/ntf/ntfd/NtfLogger.cc | 17 ++++++++++++++++- src/ntf/ntfd/NtfLogger.h | 1 + src/ntf/ntfd/NtfNotification.cc | 5 ++--- src/ntf/ntfd/NtfNotification.h | 2 +- src/ntf/ntfd/ntfd.conf | 7 +++++++ src/ntf/ntfd/ntfs.h | 4 +++- 7 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/ntf/README b/src/ntf/README index 5bf670647..ad89bb213 100644 --- a/src/ntf/README +++ b/src/ntf/README @@ -244,6 +244,15 @@ writing notification is fail for long time, NTF has to write a big number of notifications whenever handling sending notification request and that will delay to handle other requests come to NTFD. The value of variable is from 10 to 5000. +NTFSV_LOGGER_RECORD_TIMEOUT + [Thien]: This paragraph is a bit confusing. Suggest the paragraph make more clear. The logger record timeout is used to remove the overdue notification. When the file system is unresponsive and the logger buffer stays full for a long period of time, overload occurs. This variable is designed to fix the side effect of this problem like: ntf reject incoming request due to overloaded. The variable's value ranges from 5 to 30 seconds (by default, it is 10). +The logger record timeout is used to pending for logger records out of queue +after the timeout remove notification overdue. This variable is designed to +address the issue when the NTF service is busy and the logger buffer remains +full for too long, causing overload. It can be configured to adjust the lifespan +of a notification in the queue, providing more control when writing test cases +for the NTF service. The value of variable from 5s to 30s (default is 10s). + for debug see DEBUG. COMMAND LINE INTERFACE diff --git a/src/ntf/ntfd/NtfLogger.cc b/src/ntf/ntfd/NtfLogger.cc index 480c9e7b6..f6cc6de95 100644 --- a/src/ntf/ntfd/NtfLogger.cc +++ b/src/ntf/ntfd/NtfLogger.cc @@ -92,6 +92,20 @@ NtfLogger::NtfLogger() : readCounter(0), isLoggerBufferfull(false) { LOG_WA("Logger buffer is set too big, get default max instead: %d", logger_buffer_capacity); } + + /* Get the logger record timeout from the environment variable. + The value should be from 5s to 30s (Default is 10s). */ + logger_timeout_record = base::GetEnv("NTFSV_LOGGER_RECORD_TIMEOUT", + static_cast<uint32_t>(NTFSV_LOGGER_RECORD_TIMEOUT_DEFAULT)); + if (logger_timeout_record < NTFSV_LOGGER_RECORD_TIMEOUT_MIN) { + logger_timeout_record = NTFSV_LOGGER_RECORD_TIMEOUT_MIN; + LOG_WA("Logger timeout is set too small, get default min instead: %d", + NTFSV_LOGGER_RECORD_TIMEOUT_MIN); + } else if (logger_timeout_record > NTFSV_LOGGER_RECORD_TIMEOUT_MAX) { + logger_timeout_record = NTFSV_LOGGER_RECORD_TIMEOUT_MAX; + LOG_WA("Logger timeout is set too big, get default max instead: %d", + NTFSV_LOGGER_RECORD_TIMEOUT_MAX); + } } /* Callbacks */ @@ -392,7 +406,8 @@ bool NtfLogger::isExistNotification(SaInvocationT invocation) { void NtfLogger::logQueuedNotification() { if (!isLoggerBufferEmpty()) { NtfSmartPtr notification = queuedNotificationList.front(); - if (notification->is_overdue() && notification->isWaitingAck()) { + if (notification->is_overdue(logger_timeout_record) + && notification->isWaitingAck()) { LOG_NO("Notification overdue, remove notification Id: %llu", notification->getNotificationId()); dequeueNotification(); diff --git a/src/ntf/ntfd/NtfLogger.h b/src/ntf/ntfd/NtfLogger.h index 4af8fc3d8..4a3f81df7 100644 --- a/src/ntf/ntfd/NtfLogger.h +++ b/src/ntf/ntfd/NtfLogger.h @@ -83,6 +83,7 @@ class NtfLogger { typedef std::list<NtfSmartPtr> QueuedNotificationsList; QueuedNotificationsList queuedNotificationList; + uint32_t logger_timeout_record; uint32_t logger_buffer_capacity; // The flag if logger buffer is full. This is set when checking the logger diff --git a/src/ntf/ntfd/NtfNotification.cc b/src/ntf/ntfd/NtfNotification.cc index 957206dfb..213ad1b97 100644 --- a/src/ntf/ntfd/NtfNotification.cc +++ b/src/ntf/ntfd/NtfNotification.cc @@ -267,11 +267,10 @@ void NtfNotification::removeSubscription(unsigned int clientId, */ ntfsv_send_not_req_t* NtfNotification::getNotInfo() { return sendNotInfo_; } -bool NtfNotification::is_overdue() const { - uint32_t max_time = NTFSV_LOGGER_RECORD_TIMEOUT_S; +bool NtfNotification::is_overdue(uint32_t timeout) const { timespec queue_at = base::NanosToTimespec(queue_at_); timespec current = base::ReadMonotonicClock(); - timespec max_queue_time{static_cast<time_t>(max_time), 0}; + timespec max_queue_time{static_cast<time_t>(timeout), 0}; return (current - queue_at > max_queue_time); } diff --git a/src/ntf/ntfd/NtfNotification.h b/src/ntf/ntfd/NtfNotification.h index d025648cf..ec12975b9 100644 --- a/src/ntf/ntfd/NtfNotification.h +++ b/src/ntf/ntfd/NtfNotification.h @@ -68,7 +68,7 @@ class NtfNotification { SaNtfNotificationHeaderT* header(); ntfsv_send_not_req_t* sendNotInfo_; bool loggFromCallback_; - bool is_overdue() const; + bool is_overdue(uint32_t timeout) const; private: NtfNotification(); diff --git a/src/ntf/ntfd/ntfd.conf b/src/ntf/ntfd/ntfd.conf index f2f67496f..ff2379b1a 100644 --- a/src/ntf/ntfd/ntfd.conf +++ b/src/ntf/ntfd/ntfd.conf @@ -51,3 +51,10 @@ export NTFSV_ENV_HEALTHCHECK_KEY="Default" # notifications whenever handling sending notification request and that will delay # to handle other requests come to NTFD. The value of variable is from 10 to 5000. #export NTFSV_LOGGER_BUFFER_CAPACITY=10 + [Thien]: The logger record timeout is used to remove the overdue notification. +# The logger record timeout is used to pending for logger records out of queue +# after the timeout remove notification overdue. The default timeout is 10s, uncomment +# the next line to configure the logger recored timeout. The range can be configure +# from 5s to 30s. +# export NTFSV_LOGGER_RECORD_TIMEOUT=10 + diff --git a/src/ntf/ntfd/ntfs.h b/src/ntf/ntfd/ntfs.h index ee0276211..56d30dba0 100644 --- a/src/ntf/ntfd/ntfs.h +++ b/src/ntf/ntfd/ntfs.h @@ -58,7 +58,9 @@ // Periodic timer. Using in the main poll when queue available #define NTFSV_LOGGER_PERODIC_POLL_TIMEOUT_MS 5000 // Allowed queue time before notification overdue -#define NTFSV_LOGGER_RECORD_TIMEOUT_S 10 +#define NTFSV_LOGGER_RECORD_TIMEOUT_DEFAULT 10 +#define NTFSV_LOGGER_RECORD_TIMEOUT_MIN 5 +#define NTFSV_LOGGER_RECORD_TIMEOUT_MAX 30 /* ======================================================================== * TYPE DEFINITIONS -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-03-26 09:25:38
|
Hi Tai, Could you update the commit message to show your solution ideas? It should be included 2 parts in long commit message: 1) why core dump happens? 2) how to resolve in this commit? ACK with minor comments above. Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Friday, March 21, 2025 2:45 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Thang Nguyen <tha...@en...>; Vu Luu <vu...@en...>; Khuong Le <khu...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] smf: fix issue when remove node inv_id between threads [#3367] This conflict occurs when removing the node 'invid' between threads while executing the procedure in parallel. --- src/smf/smfd/SmfCallback.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/smf/smfd/SmfCallback.cc b/src/smf/smfd/SmfCallback.cc index a55d19f60..1587ef859 100644 --- a/src/smf/smfd/SmfCallback.cc +++ b/src/smf/smfd/SmfCallback.cc @@ -318,6 +318,7 @@ SaAisErrorT SmfCallback::send_callback_msg(SaSmfPhaseT phase, goto rem_invid; } free(evt); + goto done; } } rem_invid: @@ -338,6 +339,7 @@ rem_invid: } smfd_cb_unlock(); } +done: osaf_extended_name_free( &smfsv_evt.info.smfnd.event.cbk_req_rsp.evt.cbk_evt.object_name); free(smfsv_evt.info.smfnd.event.cbk_req_rsp.evt.cbk_evt.cbk_label.label); -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-03-11 09:37:57
|
Hi Tai, ACK from me. Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Monday, March 10, 2025 12:46 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Thang Nguyen <tha...@en...>; Vu Luu <vu...@en...>; Khuong Le <khu...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] mds: fix typo error [#3348] Fix incorrect use of a confused variable --- src/mds/mds_svc_op.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mds/mds_svc_op.c b/src/mds/mds_svc_op.c index 4599c0c01..5ad51eea4 100644 --- a/src/mds/mds_svc_op.c +++ b/src/mds/mds_svc_op.c @@ -642,8 +642,8 @@ uint32_t mds_svc_op_unsubscribe(const NCSMDS_INFO *info) { mds_subtn_tbl_del(svc_hdl, info->info.svc_cancel.i_svc_ids[i]); MDS_SVC_LOG_INFO(UNSUBSCRIBE_TAG, info, "Unsubscribe to svc_id = %s(%d) successful", - get_svc_names(info->info.svc_subscribe.i_svc_ids[i]), - info->info.svc_subscribe.i_svc_ids[i]); + get_svc_names(info->info.svc_cancel.i_svc_ids[i]), + info->info.svc_cancel.i_svc_ids[i]); } m_MDS_LEAVE(); return NCSCC_RC_SUCCESS; -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2025-03-11 09:10:05
|
Hi Tai, ACK from me. Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Friday, March 7, 2025 4:36 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Thang Nguyen <tha...@en...>; Vu Luu <vu...@en...>; Khuong Le <khu...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] build: Fix the invalid pylint url [#3366] Using the new url recommended by Google. --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index dc0cac1ae..15b453c3e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -408,7 +408,7 @@ cppcheck: cppcheck_includes.txt cpplint: @test -d $(top_builddir)/bin || mkdir $(top_builddir)/bin @cpplint=$$(find $(top_builddir)/bin -name cpplint.py -mtime -30); \ - test -z "$$cpplint" && wget -O $(top_builddir)/bin/cpplint.py https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fgoogle%2Fstyleguide%2Fgh-pages%2Fcpplint%2Fcpplint.py&data=05%7C02%7Cthien.m.huynh%40endava.com%7C6aba1ff37e894373d5b108dd5d5b8c15%7C0b3fc178b7304e8b9843e81259237b77%7C0%7C0%7C638769369950209353%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=A0KMEH0xOOOOaUu3UdHj07b%2BHqlaiKHEUBl69HI7ixE%3D&reserved=0<https://raw.githubusercontent.com/google/styleguide/gh-pages/cpplint/cpplint.py> 2>&1 && touch $(top_builddir)/bin/cpplint.py; \ + test -z "$$cpplint" && wget -O $(top_builddir)/bin/cpplint.py https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fcpplint%2Fcpplint%2Frefs%2Fheads%2Fdevelop%2Fcpplint.py&data=05%7C02%7Cthien.m.huynh%40endava.com%7C6aba1ff37e894373d5b108dd5d5b8c15%7C0b3fc178b7304e8b9843e81259237b77%7C0%7C0%7C638769369950227002%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=fkg8tde1w9PAyFmYC6hAyuqP7rOPMkW%2B6Qr8StWPwfg%3D&reserved=0<https://raw.githubusercontent.com/cpplint/cpplint/refs/heads/develop/cpplint.py> 2>&1 && touch $(top_builddir)/bin/cpplint.py; \ builddir=$$(cd $(top_builddir); pwd; cd - > /dev/null); \ srcdir=$$(cd $(top_srcdir); pwd; cd - > /dev/null); \ cd "$$srcdir"; find samples src \( -name '*.[CH]' -o -name '*.hh' -o -name '*.[ch]pp' -o -name '*.[ch]xx' \) -exec echo "Invalid file name: {}" \; 1>&2; \ -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-10-22 07:40:05
|
Hi Tai, ACK from me without comment. Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Thursday, October 17, 2024 2:34 PM To: Thien Huynh <thi...@en...>; Dat Phan <dat...@en...>; Thang Nguyen <tha...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] ntf: update fix thread unsafe [#3361] Only lock mutex in the search node, then it will be unblocked to avoid blocking in the destroy instance --- src/ntf/agent/ntfa.h | 3 +- src/ntf/agent/ntfa_api.c | 6 ++-- src/ntf/agent/ntfa_util.c | 75 +++++++++++++++++---------------------- 3 files changed, 35 insertions(+), 49 deletions(-) diff --git a/src/ntf/agent/ntfa.h b/src/ntf/agent/ntfa.h index 3a23afaf7..2c222749d 100644 --- a/src/ntf/agent/ntfa.h +++ b/src/ntf/agent/ntfa.h @@ -196,8 +196,7 @@ extern ntfa_notification_hdl_rec_t *ntfa_notification_hdl_rec_add( extern ntfa_filter_hdl_rec_t *ntfa_filter_hdl_rec_add( ntfa_client_hdl_rec_t **hdl_rec); extern void ntfa_hdl_list_del(ntfa_client_hdl_rec_t **); -extern uint32_t ntfa_hdl_rec_del(ntfa_client_hdl_rec_t **, - ntfa_client_hdl_rec_t *); +extern uint32_t ntfa_hdl_rec_del(ntfa_cb_t *cb, ntfa_client_hdl_rec_t *); extern void ntfa_hdl_rec_force_del(ntfa_client_hdl_rec_t **, ntfa_client_hdl_rec_t *); extern uint32_t ntfa_notification_hdl_rec_del(ntfa_notification_hdl_rec_t **, diff --git a/src/ntf/agent/ntfa_api.c b/src/ntf/agent/ntfa_api.c index 79f55714c..fabaab54b 100644 --- a/src/ntf/agent/ntfa_api.c +++ b/src/ntf/agent/ntfa_api.c @@ -1798,11 +1798,9 @@ SaAisErrorT saNtfFinalize(SaNtfHandleT ntfHandle) /** delete the hdl rec ** including all resources allocated by client if MDS send is ** succesful. + ** Note: ncshm_give_hdl handled in ntfa_hdl_rec_del **/ - ncshm_give_hdl(ntfHandle); - pthread_mutex_lock(&ntfa_cb.cb_lock); - rc = ntfa_hdl_rec_del(&ntfa_cb.client_list, hdl_rec); - pthread_mutex_unlock(&ntfa_cb.cb_lock); + rc = ntfa_hdl_rec_del(&ntfa_cb, hdl_rec); if (rc != NCSCC_RC_SUCCESS) { TRACE_1("ntfa_hdl_rec_del failed"); rc = SA_AIS_ERR_BAD_HANDLE; diff --git a/src/ntf/agent/ntfa_util.c b/src/ntf/agent/ntfa_util.c index 4d0496d20..5e61af399 100644 --- a/src/ntf/agent/ntfa_util.c +++ b/src/ntf/agent/ntfa_util.c @@ -1088,72 +1088,61 @@ void ntfa_hdl_rec_force_del(ntfa_client_hdl_rec_t **list_head, access the handle record (ie. hdl db tree or hdl mngr) is removed. This is to disallow the waiting thread to access the hdl rec while other thread executes saAmfFinalize on it. + Avoid using lock on ntfa_cb.cb_lock before destroy + the handle record in this API as it may lead to deadlock + situation in an application in which other APIs are called + along with this in parallel. ******************************************************************************/ -uint32_t ntfa_hdl_rec_del(ntfa_client_hdl_rec_t **list_head, - ntfa_client_hdl_rec_t *rm_node) +uint32_t ntfa_hdl_rec_del(ntfa_cb_t *cb, ntfa_client_hdl_rec_t *rm_node) { + ntfa_client_hdl_rec_t *list_iter = NULL; uint32_t rc = NCSCC_RC_FAILURE; - ntfa_client_hdl_rec_t *list_iter = *list_head; TRACE_ENTER(); - ncshm_give_hdl(rm_node->local_hdl); /* TODO: free all resources allocated by the client */ - /* Remove subscribers of this client if there are any in subcriberNoList + pthread_mutex_lock(&cb->cb_lock); + + /* Remove subscribers of this client if there are any in + * subcriberNoList */ ntfa_subscriber_del_by_handle(rm_node->local_hdl); - + list_iter = cb->client_list; /* If the to be removed record is the first record */ - if (list_iter == rm_node) { - *list_head = rm_node->next; - - /** detach & release the IPC - **/ - m_NCS_IPC_DETACH(&rm_node->mbx, ntfa_clear_mbx, NULL); - m_NCS_IPC_RELEASE(&rm_node->mbx, NULL); - - ncshm_destroy_hdl(NCS_SERVICE_ID_NTFA, rm_node->local_hdl); - /** Free the channel records off this hdl - **/ - ntfa_notification_hdl_rec_list_del(&rm_node->notification_list); - - /** free the hdl rec - **/ - free(rm_node); + if (cb->client_list == rm_node) { + cb->client_list = rm_node->next; rc = NCSCC_RC_SUCCESS; - goto out; } else { /* find the rec */ - while (NULL != list_iter) { if (list_iter->next == rm_node) { list_iter->next = rm_node->next; - - /** detach & release the IPC */ - m_NCS_IPC_DETACH(&rm_node->mbx, ntfa_clear_mbx, - NULL); - m_NCS_IPC_RELEASE(&rm_node->mbx, NULL); - - ncshm_destroy_hdl(NCS_SERVICE_ID_NTFA, - rm_node->local_hdl); - /** Free the channel records off this ntfa_hdl - */ - ntfa_notification_hdl_rec_list_del( - &rm_node->notification_list); - - /** free the hdl rec */ - free(rm_node); - rc = NCSCC_RC_SUCCESS; - goto out; + break; } /* move onto the next one */ list_iter = list_iter->next; } } - TRACE("failed"); + pthread_mutex_unlock(&cb->cb_lock); -out: + if (rc != NCSCC_RC_SUCCESS) { + TRACE("Not found client in list %d", rm_node->ntfs_client_id); + return rc; + } + /** detach & release the IPC + **/ + m_NCS_IPC_DETACH(&rm_node->mbx, ntfa_clear_mbx, NULL); + m_NCS_IPC_RELEASE(&rm_node->mbx, NULL); + + ncshm_destroy_hdl(NCS_SERVICE_ID_NTFA, rm_node->local_hdl); + /** Free the channel records off this hdl + **/ + ntfa_notification_hdl_rec_list_del(&rm_node->notification_list); + + /** free the hdl rec + **/ + free(rm_node); TRACE_LEAVE(); return rc; } -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-10-09 07:46:29
|
--- src/imm/immloadd/imm_loader.cc | 22 +++++++++++----------- src/imm/tools/imm_import.cc | 22 +++++++++++----------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/imm/immloadd/imm_loader.cc b/src/imm/immloadd/imm_loader.cc index fadb0da51..e0d7c6caa 100644 --- a/src/imm/immloadd/imm_loader.cc +++ b/src/imm/immloadd/imm_loader.cc @@ -994,9 +994,9 @@ static void endElementHandler(void *userData, const xmlChar *name) { char *str = (char *)malloc(1); str[0] = '\0'; - state->attrValueBuffers.push_front(str); + state->attrValueBuffers.push_back(str); } else if (state->isBase64Encoded) { - char *value = state->attrValueBuffers.front(); + char *value = state->attrValueBuffers.back(); int len = strlen(value); /* count the length of the decoded string */ @@ -1012,8 +1012,8 @@ static void endElementHandler(void *userData, const xmlChar *name) { } newvalue[newlen] = 0; - state->attrValueBuffers.pop_front(); - state->attrValueBuffers.push_front(newvalue); + state->attrValueBuffers.pop_back(); + state->attrValueBuffers.push_back(newvalue); free(value); } @@ -1390,13 +1390,13 @@ static void charactersHandler(void *userData, const xmlChar *chars, int len) { strncpy(str, (const char *)chars, (size_t)len); str[len] = '\0'; - state->attrValueBuffers.push_front(str); + state->attrValueBuffers.push_back(str); } else { /* CONTINUATION of CURRENT value, typically only happens for loooong * strings. */ TRACE_8("APPEND TO CURRENT VALUE"); - size_t oldsize = strlen(state->attrValueBuffers.front()); + size_t oldsize = strlen(state->attrValueBuffers.back()); TRACE_8("APPEND VALUE newsize:%u", (unsigned int)oldsize + len + 1); str = (char *)malloc(oldsize + len + 1); @@ -1404,7 +1404,7 @@ static void charactersHandler(void *userData, const xmlChar *chars, int len) { LOG_ER("Failed to malloc value"); exit(1); } - strncpy(str, state->attrValueBuffers.front(), oldsize + 1); + strncpy(str, state->attrValueBuffers.back(), oldsize + 1); TRACE_8("COPIED OLD VALUE %u %s", (unsigned int)oldsize, str); strncpy(str + oldsize, (const char *)chars, (size_t)len + 1); @@ -1413,13 +1413,13 @@ static void charactersHandler(void *userData, const xmlChar *chars, int len) { (unsigned int)oldsize + len + 1, str); /* Remove the old string */ - free(state->attrValueBuffers.front()); - state->attrValueBuffers.pop_front(); + free(state->attrValueBuffers.back()); + state->attrValueBuffers.pop_back(); /* state->attrValueBuffers.clear(); clear not appropriate since we could ALSO have several values! - We are here only operating on the front value in the list. + We are here only operating on the back value in the list. */ - state->attrValueBuffers.push_front(str); + state->attrValueBuffers.push_back(str); } } else { LOG_ER("UNKNOWN PLACEMENT OF VALUE"); diff --git a/src/imm/tools/imm_import.cc b/src/imm/tools/imm_import.cc index c1c178173..4b66b6b1b 100644 --- a/src/imm/tools/imm_import.cc +++ b/src/imm/tools/imm_import.cc @@ -1555,9 +1555,9 @@ static void endElementHandler(void *userData, const xmlChar *name) { char *str = (char *)malloc(1); str[0] = '\0'; - state->attrValueBuffers.push_front(str); + state->attrValueBuffers.push_back(str); } else if (state->isBase64Encoded) { - char *value = state->attrValueBuffers.front(); + char *value = state->attrValueBuffers.back(); int len = strlen(value); /* count the length of the decoded string */ @@ -1576,8 +1576,8 @@ static void endElementHandler(void *userData, const xmlChar *name) { } newvalue[newlen] = 0; - state->attrValueBuffers.pop_front(); - state->attrValueBuffers.push_front(newvalue); + state->attrValueBuffers.pop_back(); + state->attrValueBuffers.push_back(newvalue); free(value); } @@ -1836,13 +1836,13 @@ static void charactersHandler(void *userData, const xmlChar *chars, int len) { strncpy(str, (const char *)chars, (size_t)len); str[len] = '\0'; - state->attrValueBuffers.push_front(str); + state->attrValueBuffers.push_back(str); } else { /* CONTINUATION of CURRENT value, typically only happens for loooong * strings. */ TRACE_8("APPEND TO CURRENT VALUE"); - size_t oldsize = strlen(state->attrValueBuffers.front()); + size_t oldsize = strlen(state->attrValueBuffers.back()); TRACE_8("APPEND VALUE newsize:%u", oldsize + len + 1); str = (char *)malloc(oldsize + len + 1); @@ -1852,7 +1852,7 @@ static void charactersHandler(void *userData, const xmlChar *chars, int len) { state->parsingStatus = 1; return; } - strcpy(str, state->attrValueBuffers.front()); + strcpy(str, state->attrValueBuffers.back()); TRACE_8("COPIED OLD VALUE %u %s", oldsize, str); strncpy(str + oldsize, (const char *)chars, (size_t)len + 1); @@ -1860,13 +1860,13 @@ static void charactersHandler(void *userData, const xmlChar *chars, int len) { LOG_IN("APPENDED NEW VALUE newsize %u %s", oldsize + len + 1, str); /* Remove the old string */ - free(state->attrValueBuffers.front()); - state->attrValueBuffers.pop_front(); + free(state->attrValueBuffers.back()); + state->attrValueBuffers.pop_back(); /* state->attrValueBuffers.clear(); clear not appropriate since we could ALSO have several values! - We are here only operating on the front value in the list. + We are here only operating on the back value in the list. */ - state->attrValueBuffers.push_front(str); + state->attrValueBuffers.push_back(str); } } else { -- 2.46.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-10-09 07:45:29
|
Summary: imm: fix multiple value out of order when import from xml file [#3360] Review request for Ticket(s): 3360 Peer Reviewer(s): Dat, Tai, Thang Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3360 Base revision: 5a27e7b90762098c49fe1bf688b390e6270131f0 Personal repository: git://git.code.sf.net/u/thienhuynh/review -------------------------------- Impacted area Impact y/n -------------------------------- Docs n Build system n RPM/packaging n Configuration files n Startup scripts n SAF services n OpenSAF services y Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): --------------------------------------------- *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision 49ad204789ca1ae6e1c378b5088ac35ca62c9071 Author: thien.m.huynh <thi...@en...> Date: Tue, 8 Oct 2024 06:50:53 +0700 imm: fix multiple value out of order when import from xml file [#3360] Complete diffstat: ------------------ src/imm/immloadd/imm_loader.cc | 22 +++++++++++----------- src/imm/tools/imm_import.cc | 22 +++++++++++----------- 2 files changed, 22 insertions(+), 22 deletions(-) Testing Commands: ----------------- N/A Testing, Expected Results: -------------------------- N/A Conditions of Submission: ------------------------- ACK from reviewer Arch Built Started Linux distro ------------------------------------------- mips n n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: ------------------- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-09-24 10:55:58
|
Hi Tai, ACK from me. Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Tuesday, September 24, 2024 9:48 AM To: Thien Huynh <thi...@en...>; Thang Nguyen <tha...@en...>; Dat Phan <dat...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] amf: Update amf clc cli with dynamic buffer [#3358] With the large size of clc-cli will lead to coredump with buffer overflow The fix is dynamic buffer to amf clc-cli --- src/amf/amfd/comp.cc | 127 ++++++++++++++++++++---------------- src/amf/amfd/comp.h | 22 +++---- src/amf/amfd/comptype.cc | 32 +++++---- src/amf/amfnd/avnd_comp.h | 62 ++++++++++-------- src/amf/amfnd/clc.cc | 19 ++++-- src/amf/amfnd/compdb.cc | 55 +++++++++------- src/amf/common/amf_d2nmsg.h | 52 +++++++-------- src/amf/common/amf_defs.h | 3 + 8 files changed, 208 insertions(+), 164 deletions(-) diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc index 7f83bd9da..d5cdb9388 100644 --- a/src/amf/amfd/comp.cc +++ b/src/amf/amfd/comp.cc @@ -44,6 +44,45 @@ AmfDb<std::string, AVD_COMP> *comp_db = nullptr; +/** + * Initializes a single AVD CLC-CLI command for a component from comptype_cmd. + * Argument of command will be get from attrname_cmd_argv or + * comptype_def_argv. + * In the case argv of attrname_cmd_argv not exits in IMM then the command + * will get argv from comptype_def_argv. If comptype_def_argv + * also not exits then will ignore argument for command. + * @param comp_info + * @param comptype_cmd + * @param comptype_def_argv + * @param attributes + * @param attrname_cmd_argv + */ +static void init_avd_clc_cli_command(char **comp_info, + const char *comptype_cmd, + const char *comptype_def_argv, + const SaImmAttrValuesT_2 **attributes, + const char *attrname_cmd_argv) { + size_t size; + const char *str; + char *cmd_argv; + size = strlen(comptype_cmd); + size += 1; /* Increase size for SPACE */ + + if ((str = immutil_getStringAttr(attributes, attrname_cmd_argv, 0)) == + nullptr) { + str = comptype_def_argv; + } + + if (str != nullptr) size += strlen(str); + + *comp_info = (char *)calloc(size + 1, sizeof(char)); + strcpy(*comp_info, comptype_cmd); + cmd_argv = *comp_info + strlen(comptype_cmd); + *cmd_argv++ = AVSV_CHAR_SPACE; + + if (str != nullptr) strcpy(cmd_argv, str); +} + void avd_comp_db_add(AVD_COMP *comp) { const std::string comp_name(Amf::to_string(&comp->comp_info.name)); @@ -227,6 +266,11 @@ void avd_comp_delete(AVD_COMP *comp) { su->remove_comp(comp); avd_comptype_remove_comp(comp); comp_db->erase(Amf::to_string(&comp->comp_info.name)); + free(comp->comp_info.init_info); + free(comp->comp_info.term_info); + free(comp->comp_info.clean_info); + free(comp->comp_info.amstart_info); + free(comp->comp_info.amstop_info); delete comp; } @@ -1223,8 +1267,6 @@ static AVD_COMP *comp_create(const std::string &dn, const SaImmAttrValuesT_2 **attributes) { int rc = -1; AVD_COMP *comp; - char *cmd_argv; - const char *str; const AVD_COMP_TYPE *comptype; SaNameT comp_type; SaAisErrorT error; @@ -1259,17 +1301,11 @@ static AVD_COMP *comp_create(const std::string &dn, goto done; } - if (strlen(comptype->saAmfCtRelPathInstantiateCmd) > 0) { - strcpy(comp->comp_info.init_info, comptype->saAmfCtRelPathInstantiateCmd); - cmd_argv = comp->comp_info.init_info + strlen(comp->comp_info.init_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompInstantiateCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefInstantiateCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (!comptype->saAmfCtRelPathInstantiateCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.init_info, + comptype->saAmfCtRelPathInstantiateCmd.c_str(), + comptype->saAmfCtDefInstantiateCmdArgv.c_str(), + attributes, "saAmfCompInstantiateCmdArgv"); comp->comp_info.init_len = strlen(comp->comp_info.init_info); } @@ -1301,17 +1337,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->inst_retry_delay = avd_comp_global_attrs.saAmfDelayBetweenInstantiateAttempts; - if (strlen(comptype->saAmfCtRelPathTerminateCmd) > 0) { - strcpy(comp->comp_info.term_info, comptype->saAmfCtRelPathTerminateCmd); - cmd_argv = comp->comp_info.term_info + strlen(comp->comp_info.term_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompTerminateCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefTerminateCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (!comptype->saAmfCtRelPathTerminateCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.term_info, + comptype->saAmfCtRelPathTerminateCmd.c_str(), + comptype->saAmfCtDefTerminateCmdArgv.c_str(), + attributes, "saAmfCompTerminateCmdArgv"); comp->comp_info.term_len = strlen(comp->comp_info.term_info); } @@ -1321,17 +1351,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->comp_info.terminate_callback_timeout = comptype->saAmfCtDefCallbackTimeout; - if (strlen(comptype->saAmfCtRelPathCleanupCmd) > 0) { - strcpy(comp->comp_info.clean_info, comptype->saAmfCtRelPathCleanupCmd); - cmd_argv = comp->comp_info.clean_info + strlen(comp->comp_info.clean_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompCleanupCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefCleanupCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (!comptype->saAmfCtRelPathCleanupCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.clean_info, + comptype->saAmfCtRelPathCleanupCmd.c_str(), + comptype->saAmfCtDefCleanupCmdArgv.c_str(), + attributes, "saAmfCompCleanupCmdArgv"); comp->comp_info.clean_len = strlen(comp->comp_info.clean_info); } @@ -1339,18 +1363,11 @@ static AVD_COMP *comp_create(const std::string &dn, attributes, 0, &comp->comp_info.clean_time) != SA_AIS_OK) comp->comp_info.clean_time = comptype->saAmfCtDefClcCliTimeout; - if (strlen(comptype->saAmfCtRelPathAmStartCmd) > 0) { - strcpy(comp->comp_info.amstart_info, comptype->saAmfCtRelPathAmStartCmd); - cmd_argv = - comp->comp_info.amstart_info + strlen(comp->comp_info.amstart_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompAmStartCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefAmStartCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (!comptype->saAmfCtRelPathAmStartCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.amstart_info, + comptype->saAmfCtRelPathAmStartCmd.c_str(), + comptype->saAmfCtDefAmStartCmdArgv.c_str(), + attributes, "saAmfCompAmStartCmdArgv"); comp->comp_info.amstart_len = strlen(comp->comp_info.amstart_info); } @@ -1365,17 +1382,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->comp_info.max_num_amstart = avd_comp_global_attrs.saAmfNumMaxAmStartAttempts; - if (strlen(comptype->saAmfCtRelPathAmStopCmd) > 0) { - strcpy(comp->comp_info.amstop_info, comptype->saAmfCtRelPathAmStopCmd); - cmd_argv = - comp->comp_info.amstop_info + strlen(comp->comp_info.amstop_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompAmStopCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefAmStopCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); + if (!comptype->saAmfCtRelPathAmStopCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.amstop_info, + comptype->saAmfCtRelPathAmStopCmd.c_str(), + comptype->saAmfCtDefAmStopCmdArgv.c_str(), + attributes, "saAmfCompAmStopCmdArgv"); } if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCompAmStopTimeout"), diff --git a/src/amf/amfd/comp.h b/src/amf/amfd/comp.h index 4265f16d6..0ba8c3d2d 100644 --- a/src/amf/amfd/comp.h +++ b/src/amf/amfd/comp.h @@ -185,20 +185,20 @@ class AVD_COMP_TYPE { std::string name{}; SaUint32T saAmfCtCompCategory{}; std::string saAmfCtSwBundle{}; - char saAmfCtDefCmdEnv[AVSV_MISC_STR_MAX_SIZE]{}; + std::string saAmfCtDefCmdEnv{}; SaTimeT saAmfCtDefClcCliTimeout{}; SaTimeT saAmfCtDefCallbackTimeout{}; - char saAmfCtRelPathInstantiateCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefInstantiateCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; + std::string saAmfCtRelPathInstantiateCmd{}; + std::string saAmfCtDefInstantiateCmdArgv{}; SaUint32T saAmfCtDefInstantiationLevel{}; - char saAmfCtRelPathTerminateCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefTerminateCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathCleanupCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefCleanupCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathAmStartCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefAmStartCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathAmStopCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefAmStopCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; + std::string saAmfCtRelPathTerminateCmd{}; + std::string saAmfCtDefTerminateCmdArgv{}; + std::string saAmfCtRelPathCleanupCmd{}; + std::string saAmfCtDefCleanupCmdArgv{}; + std::string saAmfCtRelPathAmStartCmd{}; + std::string saAmfCtDefAmStartCmdArgv{}; + std::string saAmfCtRelPathAmStopCmd{}; + std::string saAmfCtDefAmStopCmdArgv{}; SaTimeT saAmfCtDefQuiescingCompleteTimeout{}; SaAmfRecommendedRecoveryT saAmfCtDefRecoveryOnError{}; SaBoolT saAmfCtDefDisableRestart{}; diff --git a/src/amf/amfd/comptype.cc b/src/amf/amfd/comptype.cc index 48a333e8c..178209baf 100644 --- a/src/amf/amfd/comptype.cc +++ b/src/amf/amfd/comptype.cc @@ -95,9 +95,11 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtSwBundle"), attributes, 0, &ct_sw_bundle); compt->saAmfCtSwBundle = Amf::to_string(&ct_sw_bundle); + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefCmdEnv", 0)) != nullptr) - strcpy(compt->saAmfCtDefCmdEnv, str); + compt->saAmfCtDefCmdEnv = str; + (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtDefClcCliTimeout"), attributes, 0, &compt->saAmfCtDefClcCliTimeout); (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtDefCallbackTimeout"), @@ -105,10 +107,11 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathInstantiateCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathInstantiateCmd, str); + compt->saAmfCtRelPathInstantiateCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefInstantiateCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefInstantiateCmdArgv, str); + compt->saAmfCtDefInstantiateCmdArgv = str; (void)immutil_getAttr( const_cast<SaImmAttrNameT>("saAmfCtDefInstantiationLevel"), attributes, 0, @@ -116,28 +119,35 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathTerminateCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathTerminateCmd, str); + compt->saAmfCtRelPathTerminateCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefTerminateCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefTerminateCmdArgv, str); + compt->saAmfCtDefTerminateCmdArgv = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathCleanupCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathCleanupCmd, str); + compt->saAmfCtRelPathCleanupCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefCleanupCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefCleanupCmdArgv, str); + compt->saAmfCtDefCleanupCmdArgv = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathAmStartCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathAmStartCmd, str); + compt->saAmfCtRelPathAmStartCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefAmStartCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefAmStartCmdArgv, str); + compt->saAmfCtDefAmStartCmdArgv = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathAmStopCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathAmStopCmd, str); + compt->saAmfCtRelPathAmStopCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefAmStopCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefAmStopCmdArgv, str); + compt->saAmfCtDefAmStopCmdArgv = str; if ((IS_COMP_SAAWARE(compt->saAmfCtCompCategory) || IS_COMP_PROXIED_PI(compt->saAmfCtCompCategory) || diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h index c3339ac79..a986d8a43 100644 --- a/src/amf/amfnd/avnd_comp.h +++ b/src/amf/amfnd/avnd_comp.h @@ -91,9 +91,9 @@ typedef enum avnd_comp_clc_cmd_type { /* clc command parameter definition */ typedef struct avnd_comp_clc_param { - char cmd[SAAMF_CLC_LEN]; /* cmd ascii string */ - SaTimeT timeout; /* cmd timeout value */ - uint32_t len; /* cmd len */ + std::string cmd; /* cmd ascii string */ + SaTimeT timeout; /* cmd timeout value */ + uint32_t len; /* cmd len */ } AVND_COMP_CLC_CMD_PARAM; /* clc info definition (top level wrapper structure) */ @@ -691,31 +691,39 @@ typedef struct avnd_comp_tag { void m_AVND_COMP_OPER_STATE_AVD_SYNC(struct avnd_cb_tag *cb, const AVND_COMP *comp, uint32_t &o_rc); +/* marco count argc the clc cmd string */ +#define m_AVND_COMP_CLC_COUNT_AGRC(clc_cmd, len, count) \ + { \ + uint32_t i = 0; \ + (count)++; \ + while (i < (len)) { \ + if ((clc_cmd)[i++] == AVSV_CHAR_SPACE) { \ + (count)++; \ + while ((clc_cmd)[i++] == AVSV_CHAR_SPACE); \ + } \ + } \ + if ((clc_cmd)[0] == AVSV_CHAR_SPACE) (count)--; \ + if ((clc_cmd)[(len) - 1] == AVSV_CHAR_SPACE) (count)--; \ + } + /* macro to parse the clc cmd string */ -#define m_AVND_COMP_CLC_STR_PARSE(st, sc, ac, av, tav) \ - { \ - char str[SAAMF_CLC_LEN], *tok = nullptr; \ - /* copy the str as strtok modifies the original str */ \ - strcpy(str, st); \ - ac = 0; \ - if (nullptr != (tok = strtok(str, " "))) { \ - strncpy(sc, tok, SAAMF_CLC_LEN - 1); \ - av[ac] = sc; \ - } \ - ac++; \ - while ((nullptr != (tok = strtok(nullptr, " "))) && \ - (ac < (AVND_COMP_CLC_PARAM_MAX + 1))) { \ - if (strlen(tok) > AVND_COMP_CLC_PARAM_SIZE_MAX) break; \ - strcpy(tav[ac], tok); \ - av[ac] = tav[ac]; \ - ac++; \ - } \ - if (nullptr != tok) { \ - sc[0] = (char)(long)nullptr; \ - av[0] = nullptr; \ - ac = 0; \ - } else \ - av[ac] = nullptr; \ +#define m_AVND_COMP_CLC_STR_PARSE(sc, av) \ + { \ + char *tok = nullptr; \ + uint32_t ac = 0; \ + if (nullptr != (tok = strtok(sc, " "))) { \ + (av)[ac] = strdup(tok); \ + } \ + ac = 1; \ + while ((nullptr != (tok = strtok(nullptr, " ")))) { \ + (av)[ac] = strdup(tok); \ + ac++; \ + } \ + if (nullptr != tok) { \ + (av)[0] = nullptr; \ + ac = 0; \ + } else \ + (av)[ac] = nullptr; \ } /* macros for comp proxy status */ diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc index 3c35a916f..3a196211a 100644 --- a/src/amf/amfnd/clc.cc +++ b/src/amf/amfnd/clc.cc @@ -291,12 +291,13 @@ static void log_failed_exec(NCS_OS_PROC_EXEC_STATUS_INFO *exec_stat, LOG_NO("Exit code: %u", exec_stat->info.exit_with_code.exit_code); if (NCS_OS_PROC_EXEC_FAIL == exec_stat->value) - LOG_NO("CLC CLI script:'%s'", comp->clc_info.cmds[exec_cmd - 1].cmd); + LOG_NO("CLC CLI script:'%s'", + comp->clc_info.cmds[exec_cmd - 1].cmd.c_str()); if (NCS_OS_PROC_EXIT_ON_SIGNAL == exec_stat->value) LOG_NO("Signal: %u, CLC CLI script:'%s'", exec_stat->info.exit_on_signal.signal_num, - comp->clc_info.cmds[exec_cmd - 1].cmd); + comp->clc_info.cmds[exec_cmd - 1].cmd.c_str()); } /**************************************************************************** @@ -3107,10 +3108,10 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, AVND_CLC_EVT *clc_evt; AVND_EVT *evt = 0; AVND_COMP_CLC_INFO *clc_info = &comp->clc_info; - char scr[SAAMF_CLC_LEN]; - char *argv[AVND_COMP_CLC_PARAM_MAX + 2]; - char tmp_argv[AVND_COMP_CLC_PARAM_MAX + 2][AVND_COMP_CLC_PARAM_SIZE_MAX]; + char *scr = strdup(comp->clc_info.cmds[cmd_type - 1].cmd.c_str()); uint32_t argc = 0, rc = NCSCC_RC_SUCCESS, count = 0; + m_AVND_COMP_CLC_COUNT_AGRC(scr, comp->clc_info.cmds[cmd_type - 1].len, argc); + char *argv[argc + 2]; unsigned int env_counter; unsigned int i; SaStringT env; @@ -3283,8 +3284,7 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, arg.env_arg = env_set; /* tokenize the cmd */ - m_AVND_COMP_CLC_STR_PARSE(clc_info->cmds[cmd_type - 1].cmd, scr, argc, argv, - tmp_argv); + m_AVND_COMP_CLC_STR_PARSE(scr, argv); /* populate the cmd-info */ cmd_info.i_script = argv[0]; @@ -3318,6 +3318,10 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, } free(env_set); + for (count = 0; count < argc; count++) { + free(argv[count]); + } + if (NCSCC_RC_SUCCESS != rc) { TRACE_2("The CLC CLI command execution failed"); /* generate a cmd failure event; it'll be executed asynchronously */ @@ -3350,6 +3354,7 @@ err: if (evt) avnd_evt_destroy(evt); done: + free(scr); TRACE_LEAVE2("%u", rc); return rc; } diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc index 991a69214..b31e15650 100644 --- a/src/amf/amfnd/compdb.cc +++ b/src/amf/amfnd/compdb.cc @@ -1134,36 +1134,43 @@ done: * @param attributes * @param attr_name */ -static void init_clc_cli_command(AVND_COMP_CLC_CMD_PARAM *cmd, +static void init_clc_cli_command(AVND_COMP_CLC_CMD_PARAM **cmd, const char *clc_cmd, char **clc_cmd_argv, const char *path_prefix, const SaImmAttrValuesT_2 **attributes, const char *attr_name) { - char *buf = cmd->cmd; - int i, j; + int i = 0; const char *argv; + /* Clear string in case re-init command */ + if (!(*cmd)->cmd.empty()) { + (*cmd)->cmd.clear(); + } // prepend with path prefix if available - if (path_prefix == nullptr) - i = snprintf(buf, sizeof(cmd->cmd), "%s", clc_cmd); - else - i = snprintf(buf, sizeof(cmd->cmd), "%s/%s", path_prefix, clc_cmd); + if (path_prefix == nullptr) { + (*cmd)->cmd.append(clc_cmd); + } else { + (*cmd)->cmd.append(path_prefix); + (*cmd)->cmd.push_back('/'); + (*cmd)->cmd.append(clc_cmd); + } // append argv from comp type - j = 0; - while ((argv = clc_cmd_argv[j++]) != nullptr) - i += snprintf(&buf[i], sizeof(cmd->cmd) - i, " %s", argv); + while ((argv = clc_cmd_argv[i++]) != nullptr) { + (*cmd)->cmd.push_back(' '); + (*cmd)->cmd.append(argv); + } // append argv from comp instance - j = 0; - while ((argv = immutil_getStringAttr(attributes, attr_name, j++)) != nullptr) - i += snprintf(&buf[i], sizeof(cmd->cmd) - i, " %s", argv); - - cmd->len = i; + i = 0; + while ((argv = immutil_getStringAttr(attributes, attr_name, i++)) != + nullptr) { + (*cmd)->cmd.push_back(' '); + (*cmd)->cmd.append(argv); + } - /* Check for truncation, should alloc these strings dynamically instead */ - osafassert((cmd->len > 0) && (cmd->len < sizeof(cmd->cmd))); - TRACE("cmd=%s", cmd->cmd); + (*cmd)->len = (*cmd)->cmd.length(); + TRACE("cmd=%s", (*cmd)->cmd.c_str()); } /** @@ -1184,7 +1191,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_INSTANTIATE - 1]; if (comptype->saAmfCtRelPathInstantiateCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathInstantiateCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathInstantiateCmd, comptype->saAmfCtDefInstantiateCmdArgv, path_prefix, attributes, "saAmfCompInstantiateCmdArgv"); } @@ -1201,7 +1208,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1]; if (comptype->saAmfCtRelPathTerminateCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathTerminateCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathTerminateCmd, comptype->saAmfCtDefTerminateCmdArgv, path_prefix, attributes, "saAmfCompTerminateCmdArgv"); } @@ -1222,7 +1229,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP - 1]; if (comptype->saAmfCtRelPathCleanupCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathCleanupCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathCleanupCmd, comptype->saAmfCtDefCleanupCmdArgv, path_prefix, attributes, "saAmfCompCleanupCmdArgv"); } @@ -1239,7 +1246,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_AMSTART - 1]; if (comptype->saAmfCtRelPathAmStartCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathAmStartCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathAmStartCmd, comptype->saAmfCtDefAmStartCmdArgv, path_prefix, attributes, "saAmfCompAmStartCmdArgv"); comp->is_am_en = true; @@ -1253,7 +1260,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_AMSTOP - 1]; if (comptype->saAmfCtRelPathAmStopCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathAmStopCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathAmStopCmd, comptype->saAmfCtDefAmStopCmdArgv, path_prefix, attributes, "saAmfCompAmStopCmdArgv"); } @@ -1266,7 +1273,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_HC - 1]; if (comptype->osafAmfCtRelPathHcCmd != nullptr) { - init_clc_cli_command(cmd, comptype->osafAmfCtRelPathHcCmd, + init_clc_cli_command(&cmd, comptype->osafAmfCtRelPathHcCmd, comptype->osafAmfCtDefHcCmdArgv, path_prefix, attributes, "osafAmfCompHcCmdArgv"); comp->is_hc_cmd_configured = true; diff --git a/src/amf/common/amf_d2nmsg.h b/src/amf/common/amf_d2nmsg.h index 957d3a326..9122325b3 100644 --- a/src/amf/common/amf_d2nmsg.h +++ b/src/amf/common/amf_d2nmsg.h @@ -166,11 +166,11 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char init_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for - * initialization of component - * Checkpointing - Sent as a one time - * update. - */ + char *init_info; /* ASCII string of information for + * initialization of component + * Checkpointing - Sent as a one time + * update. + */ char init_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT init_time; /* Time interval within which @@ -184,11 +184,11 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char term_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for - * termination of component - * Checkpointing - Sent as a one time - * update. - */ + char *term_info; /* ASCII string of information for + * termination of component + * Checkpointing - Sent as a one time + * update. + */ char term_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT term_time; /* Time interval within which @@ -202,10 +202,10 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char clean_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for - * cleanup of component Checkpointing - * - Sent as a one time update. - */ + char *clean_info; /* ASCII string of information for + * cleanup of component Checkpointing + * - Sent as a one time update. + */ char clean_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT clean_time; /* Time interval within which @@ -219,12 +219,12 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char amstart_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII - * string of information for - * AM start of a component - * Checkpointing - Sent as a one - * time update. - */ + char *amstart_info; /* ASCII + * string of information for + * AM start of a component + * Checkpointing - Sent as a one + * time update. + */ char amstart_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT amstart_time; /* Time interval within which @@ -238,12 +238,12 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char amstop_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII - * string of information for - * AM start of a component. - * Checkpointing - Sent as a one - * time update. - */ + char *amstop_info; /* ASCII + * string of information for + * AM start of a component. + * Checkpointing - Sent as a one + * time update. + */ char amstop_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT amstop_time; /* Time interval within which diff --git a/src/amf/common/amf_defs.h b/src/amf/common/amf_defs.h index 3ee5a5aca..cbc4b8261 100644 --- a/src/amf/common/amf_defs.h +++ b/src/amf/common/amf_defs.h @@ -49,6 +49,9 @@ */ #define AVSV_MISC_STR_MAX_SIZE 256 +/* The define for character SPACE*/ +#define AVSV_CHAR_SPACE 0x20 + /* Minimum preferred num of su in 2N, N+M and NWay red model*/ #define AVSV_SG_2N_PREF_INSVC_SU_MIN 2 -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-09-23 10:14:27
|
Hi Tai, ACK with a minor comment. Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Monday, September 23, 2024 4:42 PM To: Thien Huynh <thi...@en...>; Thang Nguyen <tha...@en...>; Dat Phan <dat...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] amf: Update amf clc cli with dynamic buffer [#3358] With the large size of clc-cli will lead to coredump with buffer overflow The fix is dynamic buffer to amf clc-cli --- src/amf/amfd/comp.cc | 127 ++++++++++++++++++++---------------- src/amf/amfd/comp.h | 22 +++---- src/amf/amfd/comptype.cc | 32 +++++---- src/amf/amfnd/avnd_comp.h | 62 ++++++++++-------- src/amf/amfnd/clc.cc | 19 ++++-- src/amf/amfnd/compdb.cc | 55 +++++++++------- src/amf/common/amf_d2nmsg.h | 52 +++++++-------- src/amf/common/amf_defs.h | 3 + 8 files changed, 208 insertions(+), 164 deletions(-) diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc index 7f83bd9da..a5f1e4b0c 100644 --- a/src/amf/amfd/comp.cc +++ b/src/amf/amfd/comp.cc @@ -44,6 +44,45 @@ AmfDb<std::string, AVD_COMP> *comp_db = nullptr; +/** + * Initializes a single AVD CLC-CLI command for a component from comptype_cmd. + * Argument of command will be get from attrname_cmd_argv or + * comptype_def_argv. + * In the case argv of attrname_cmd_argv not exits in IMM then the command + * will get argv from comptype_def_argv. If comptype_def_argv + * also not exits then will irgnore argument for command. [Thien] Correct typo "irgnore" + * @param comp_info + * @param comptype_cmd + * @param comptype_def_argv + * @param attributes + * @param attrname_cmd_argv + */ +static void init_avd_clc_cli_command(char **comp_info, + const char *comptype_cmd, + const char *comptype_def_argv, + const SaImmAttrValuesT_2 **attributes, + const char *attrname_cmd_argv) { + size_t size; + const char *str; + char *cmd_argv; + size = strlen(comptype_cmd); + size += 1; /* Increase size for SPACE */ + + if ((str = immutil_getStringAttr(attributes, attrname_cmd_argv, 0)) == + nullptr) { + str = comptype_def_argv; + } + + if (str != nullptr) size += strlen(str); + + *comp_info = (char *)calloc(size + 1, sizeof(char)); + strcpy(*comp_info, comptype_cmd); + cmd_argv = *comp_info + strlen(comptype_cmd); + *cmd_argv++ = AVSV_CHAR_SPACE; + + if (str != nullptr) strcpy(cmd_argv, str); +} + void avd_comp_db_add(AVD_COMP *comp) { const std::string comp_name(Amf::to_string(&comp->comp_info.name)); @@ -227,6 +266,11 @@ void avd_comp_delete(AVD_COMP *comp) { su->remove_comp(comp); avd_comptype_remove_comp(comp); comp_db->erase(Amf::to_string(&comp->comp_info.name)); + free(comp->comp_info.init_info); + free(comp->comp_info.term_info); + free(comp->comp_info.clean_info); + free(comp->comp_info.amstart_info); + free(comp->comp_info.amstop_info); delete comp; } @@ -1223,8 +1267,6 @@ static AVD_COMP *comp_create(const std::string &dn, const SaImmAttrValuesT_2 **attributes) { int rc = -1; AVD_COMP *comp; - char *cmd_argv; - const char *str; const AVD_COMP_TYPE *comptype; SaNameT comp_type; SaAisErrorT error; @@ -1259,17 +1301,11 @@ static AVD_COMP *comp_create(const std::string &dn, goto done; } - if (strlen(comptype->saAmfCtRelPathInstantiateCmd) > 0) { - strcpy(comp->comp_info.init_info, comptype->saAmfCtRelPathInstantiateCmd); - cmd_argv = comp->comp_info.init_info + strlen(comp->comp_info.init_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompInstantiateCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefInstantiateCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (!comptype->saAmfCtRelPathInstantiateCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.init_info, + comptype->saAmfCtRelPathInstantiateCmd.c_str(), + comptype->saAmfCtDefInstantiateCmdArgv.c_str(), + attributes, "saAmfCompInstantiateCmdArgv"); comp->comp_info.init_len = strlen(comp->comp_info.init_info); } @@ -1301,17 +1337,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->inst_retry_delay = avd_comp_global_attrs.saAmfDelayBetweenInstantiateAttempts; - if (strlen(comptype->saAmfCtRelPathTerminateCmd) > 0) { - strcpy(comp->comp_info.term_info, comptype->saAmfCtRelPathTerminateCmd); - cmd_argv = comp->comp_info.term_info + strlen(comp->comp_info.term_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompTerminateCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefTerminateCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (!comptype->saAmfCtRelPathTerminateCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.term_info, + comptype->saAmfCtRelPathTerminateCmd.c_str(), + comptype->saAmfCtDefTerminateCmdArgv.c_str(), + attributes, "saAmfCompTerminateCmdArgv"); comp->comp_info.term_len = strlen(comp->comp_info.term_info); } @@ -1321,17 +1351,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->comp_info.terminate_callback_timeout = comptype->saAmfCtDefCallbackTimeout; - if (strlen(comptype->saAmfCtRelPathCleanupCmd) > 0) { - strcpy(comp->comp_info.clean_info, comptype->saAmfCtRelPathCleanupCmd); - cmd_argv = comp->comp_info.clean_info + strlen(comp->comp_info.clean_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompCleanupCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefCleanupCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (!comptype->saAmfCtRelPathCleanupCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.clean_info, + comptype->saAmfCtRelPathCleanupCmd.c_str(), + comptype->saAmfCtDefCleanupCmdArgv.c_str(), + attributes, "saAmfCompCleanupCmdArgv"); comp->comp_info.clean_len = strlen(comp->comp_info.clean_info); } @@ -1339,18 +1363,11 @@ static AVD_COMP *comp_create(const std::string &dn, attributes, 0, &comp->comp_info.clean_time) != SA_AIS_OK) comp->comp_info.clean_time = comptype->saAmfCtDefClcCliTimeout; - if (strlen(comptype->saAmfCtRelPathAmStartCmd) > 0) { - strcpy(comp->comp_info.amstart_info, comptype->saAmfCtRelPathAmStartCmd); - cmd_argv = - comp->comp_info.amstart_info + strlen(comp->comp_info.amstart_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompAmStartCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefAmStartCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (!comptype->saAmfCtRelPathAmStartCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.amstart_info, + comptype->saAmfCtRelPathAmStartCmd.c_str(), + comptype->saAmfCtDefAmStartCmdArgv.c_str(), + attributes, "saAmfCompAmStartCmdArgv"); comp->comp_info.amstart_len = strlen(comp->comp_info.amstart_info); } @@ -1365,17 +1382,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->comp_info.max_num_amstart = avd_comp_global_attrs.saAmfNumMaxAmStartAttempts; - if (strlen(comptype->saAmfCtRelPathAmStopCmd) > 0) { - strcpy(comp->comp_info.amstop_info, comptype->saAmfCtRelPathAmStopCmd); - cmd_argv = - comp->comp_info.amstop_info + strlen(comp->comp_info.amstop_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompAmStopCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefAmStopCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); + if (!comptype->saAmfCtRelPathAmStopCmd.empty()) { + init_avd_clc_cli_command(&comp->comp_info.amstop_info, + comptype->saAmfCtRelPathAmStopCmd.c_str(), + comptype->saAmfCtDefAmStopCmdArgv.c_str(), + attributes, "saAmfCompAmStopCmdArgv"); } if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCompAmStopTimeout"), diff --git a/src/amf/amfd/comp.h b/src/amf/amfd/comp.h index 4265f16d6..0ba8c3d2d 100644 --- a/src/amf/amfd/comp.h +++ b/src/amf/amfd/comp.h @@ -185,20 +185,20 @@ class AVD_COMP_TYPE { std::string name{}; SaUint32T saAmfCtCompCategory{}; std::string saAmfCtSwBundle{}; - char saAmfCtDefCmdEnv[AVSV_MISC_STR_MAX_SIZE]{}; + std::string saAmfCtDefCmdEnv{}; SaTimeT saAmfCtDefClcCliTimeout{}; SaTimeT saAmfCtDefCallbackTimeout{}; - char saAmfCtRelPathInstantiateCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefInstantiateCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; + std::string saAmfCtRelPathInstantiateCmd{}; + std::string saAmfCtDefInstantiateCmdArgv{}; SaUint32T saAmfCtDefInstantiationLevel{}; - char saAmfCtRelPathTerminateCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefTerminateCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathCleanupCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefCleanupCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathAmStartCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefAmStartCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathAmStopCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefAmStopCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; + std::string saAmfCtRelPathTerminateCmd{}; + std::string saAmfCtDefTerminateCmdArgv{}; + std::string saAmfCtRelPathCleanupCmd{}; + std::string saAmfCtDefCleanupCmdArgv{}; + std::string saAmfCtRelPathAmStartCmd{}; + std::string saAmfCtDefAmStartCmdArgv{}; + std::string saAmfCtRelPathAmStopCmd{}; + std::string saAmfCtDefAmStopCmdArgv{}; SaTimeT saAmfCtDefQuiescingCompleteTimeout{}; SaAmfRecommendedRecoveryT saAmfCtDefRecoveryOnError{}; SaBoolT saAmfCtDefDisableRestart{}; diff --git a/src/amf/amfd/comptype.cc b/src/amf/amfd/comptype.cc index 48a333e8c..178209baf 100644 --- a/src/amf/amfd/comptype.cc +++ b/src/amf/amfd/comptype.cc @@ -95,9 +95,11 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtSwBundle"), attributes, 0, &ct_sw_bundle); compt->saAmfCtSwBundle = Amf::to_string(&ct_sw_bundle); + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefCmdEnv", 0)) != nullptr) - strcpy(compt->saAmfCtDefCmdEnv, str); + compt->saAmfCtDefCmdEnv = str; + (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtDefClcCliTimeout"), attributes, 0, &compt->saAmfCtDefClcCliTimeout); (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtDefCallbackTimeout"), @@ -105,10 +107,11 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathInstantiateCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathInstantiateCmd, str); + compt->saAmfCtRelPathInstantiateCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefInstantiateCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefInstantiateCmdArgv, str); + compt->saAmfCtDefInstantiateCmdArgv = str; (void)immutil_getAttr( const_cast<SaImmAttrNameT>("saAmfCtDefInstantiationLevel"), attributes, 0, @@ -116,28 +119,35 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathTerminateCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathTerminateCmd, str); + compt->saAmfCtRelPathTerminateCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefTerminateCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefTerminateCmdArgv, str); + compt->saAmfCtDefTerminateCmdArgv = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathCleanupCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathCleanupCmd, str); + compt->saAmfCtRelPathCleanupCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefCleanupCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefCleanupCmdArgv, str); + compt->saAmfCtDefCleanupCmdArgv = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathAmStartCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathAmStartCmd, str); + compt->saAmfCtRelPathAmStartCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefAmStartCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefAmStartCmdArgv, str); + compt->saAmfCtDefAmStartCmdArgv = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathAmStopCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathAmStopCmd, str); + compt->saAmfCtRelPathAmStopCmd = str; + if ((str = immutil_getStringAttr(attributes, "saAmfCtDefAmStopCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefAmStopCmdArgv, str); + compt->saAmfCtDefAmStopCmdArgv = str; if ((IS_COMP_SAAWARE(compt->saAmfCtCompCategory) || IS_COMP_PROXIED_PI(compt->saAmfCtCompCategory) || diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h index c3339ac79..a986d8a43 100644 --- a/src/amf/amfnd/avnd_comp.h +++ b/src/amf/amfnd/avnd_comp.h @@ -91,9 +91,9 @@ typedef enum avnd_comp_clc_cmd_type { /* clc command parameter definition */ typedef struct avnd_comp_clc_param { - char cmd[SAAMF_CLC_LEN]; /* cmd ascii string */ - SaTimeT timeout; /* cmd timeout value */ - uint32_t len; /* cmd len */ + std::string cmd; /* cmd ascii string */ + SaTimeT timeout; /* cmd timeout value */ + uint32_t len; /* cmd len */ } AVND_COMP_CLC_CMD_PARAM; /* clc info definition (top level wrapper structure) */ @@ -691,31 +691,39 @@ typedef struct avnd_comp_tag { void m_AVND_COMP_OPER_STATE_AVD_SYNC(struct avnd_cb_tag *cb, const AVND_COMP *comp, uint32_t &o_rc); +/* marco count argc the clc cmd string */ +#define m_AVND_COMP_CLC_COUNT_AGRC(clc_cmd, len, count) \ + { \ + uint32_t i = 0; \ + (count)++; \ + while (i < (len)) { \ + if ((clc_cmd)[i++] == AVSV_CHAR_SPACE) { \ + (count)++; \ + while ((clc_cmd)[i++] == AVSV_CHAR_SPACE); \ + } \ + } \ + if ((clc_cmd)[0] == AVSV_CHAR_SPACE) (count)--; \ + if ((clc_cmd)[(len) - 1] == AVSV_CHAR_SPACE) (count)--; \ + } + /* macro to parse the clc cmd string */ -#define m_AVND_COMP_CLC_STR_PARSE(st, sc, ac, av, tav) \ - { \ - char str[SAAMF_CLC_LEN], *tok = nullptr; \ - /* copy the str as strtok modifies the original str */ \ - strcpy(str, st); \ - ac = 0; \ - if (nullptr != (tok = strtok(str, " "))) { \ - strncpy(sc, tok, SAAMF_CLC_LEN - 1); \ - av[ac] = sc; \ - } \ - ac++; \ - while ((nullptr != (tok = strtok(nullptr, " "))) && \ - (ac < (AVND_COMP_CLC_PARAM_MAX + 1))) { \ - if (strlen(tok) > AVND_COMP_CLC_PARAM_SIZE_MAX) break; \ - strcpy(tav[ac], tok); \ - av[ac] = tav[ac]; \ - ac++; \ - } \ - if (nullptr != tok) { \ - sc[0] = (char)(long)nullptr; \ - av[0] = nullptr; \ - ac = 0; \ - } else \ - av[ac] = nullptr; \ +#define m_AVND_COMP_CLC_STR_PARSE(sc, av) \ + { \ + char *tok = nullptr; \ + uint32_t ac = 0; \ + if (nullptr != (tok = strtok(sc, " "))) { \ + (av)[ac] = strdup(tok); \ + } \ + ac = 1; \ + while ((nullptr != (tok = strtok(nullptr, " ")))) { \ + (av)[ac] = strdup(tok); \ + ac++; \ + } \ + if (nullptr != tok) { \ + (av)[0] = nullptr; \ + ac = 0; \ + } else \ + (av)[ac] = nullptr; \ } /* macros for comp proxy status */ diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc index 3c35a916f..3a196211a 100644 --- a/src/amf/amfnd/clc.cc +++ b/src/amf/amfnd/clc.cc @@ -291,12 +291,13 @@ static void log_failed_exec(NCS_OS_PROC_EXEC_STATUS_INFO *exec_stat, LOG_NO("Exit code: %u", exec_stat->info.exit_with_code.exit_code); if (NCS_OS_PROC_EXEC_FAIL == exec_stat->value) - LOG_NO("CLC CLI script:'%s'", comp->clc_info.cmds[exec_cmd - 1].cmd); + LOG_NO("CLC CLI script:'%s'", + comp->clc_info.cmds[exec_cmd - 1].cmd.c_str()); if (NCS_OS_PROC_EXIT_ON_SIGNAL == exec_stat->value) LOG_NO("Signal: %u, CLC CLI script:'%s'", exec_stat->info.exit_on_signal.signal_num, - comp->clc_info.cmds[exec_cmd - 1].cmd); + comp->clc_info.cmds[exec_cmd - 1].cmd.c_str()); } /**************************************************************************** @@ -3107,10 +3108,10 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, AVND_CLC_EVT *clc_evt; AVND_EVT *evt = 0; AVND_COMP_CLC_INFO *clc_info = &comp->clc_info; - char scr[SAAMF_CLC_LEN]; - char *argv[AVND_COMP_CLC_PARAM_MAX + 2]; - char tmp_argv[AVND_COMP_CLC_PARAM_MAX + 2][AVND_COMP_CLC_PARAM_SIZE_MAX]; + char *scr = strdup(comp->clc_info.cmds[cmd_type - 1].cmd.c_str()); uint32_t argc = 0, rc = NCSCC_RC_SUCCESS, count = 0; + m_AVND_COMP_CLC_COUNT_AGRC(scr, comp->clc_info.cmds[cmd_type - 1].len, argc); + char *argv[argc + 2]; unsigned int env_counter; unsigned int i; SaStringT env; @@ -3283,8 +3284,7 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, arg.env_arg = env_set; /* tokenize the cmd */ - m_AVND_COMP_CLC_STR_PARSE(clc_info->cmds[cmd_type - 1].cmd, scr, argc, argv, - tmp_argv); + m_AVND_COMP_CLC_STR_PARSE(scr, argv); /* populate the cmd-info */ cmd_info.i_script = argv[0]; @@ -3318,6 +3318,10 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, } free(env_set); + for (count = 0; count < argc; count++) { + free(argv[count]); + } + if (NCSCC_RC_SUCCESS != rc) { TRACE_2("The CLC CLI command execution failed"); /* generate a cmd failure event; it'll be executed asynchronously */ @@ -3350,6 +3354,7 @@ err: if (evt) avnd_evt_destroy(evt); done: + free(scr); TRACE_LEAVE2("%u", rc); return rc; } diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc index 991a69214..b31e15650 100644 --- a/src/amf/amfnd/compdb.cc +++ b/src/amf/amfnd/compdb.cc @@ -1134,36 +1134,43 @@ done: * @param attributes * @param attr_name */ -static void init_clc_cli_command(AVND_COMP_CLC_CMD_PARAM *cmd, +static void init_clc_cli_command(AVND_COMP_CLC_CMD_PARAM **cmd, const char *clc_cmd, char **clc_cmd_argv, const char *path_prefix, const SaImmAttrValuesT_2 **attributes, const char *attr_name) { - char *buf = cmd->cmd; - int i, j; + int i = 0; const char *argv; + /* Clear string in case re-init command */ + if (!(*cmd)->cmd.empty()) { + (*cmd)->cmd.clear(); + } // prepend with path prefix if available - if (path_prefix == nullptr) - i = snprintf(buf, sizeof(cmd->cmd), "%s", clc_cmd); - else - i = snprintf(buf, sizeof(cmd->cmd), "%s/%s", path_prefix, clc_cmd); + if (path_prefix == nullptr) { + (*cmd)->cmd.append(clc_cmd); + } else { + (*cmd)->cmd.append(path_prefix); + (*cmd)->cmd.push_back('/'); + (*cmd)->cmd.append(clc_cmd); + } // append argv from comp type - j = 0; - while ((argv = clc_cmd_argv[j++]) != nullptr) - i += snprintf(&buf[i], sizeof(cmd->cmd) - i, " %s", argv); + while ((argv = clc_cmd_argv[i++]) != nullptr) { + (*cmd)->cmd.push_back(' '); + (*cmd)->cmd.append(argv); + } // append argv from comp instance - j = 0; - while ((argv = immutil_getStringAttr(attributes, attr_name, j++)) != nullptr) - i += snprintf(&buf[i], sizeof(cmd->cmd) - i, " %s", argv); - - cmd->len = i; + i = 0; + while ((argv = immutil_getStringAttr(attributes, attr_name, i++)) != + nullptr) { + (*cmd)->cmd.push_back(' '); + (*cmd)->cmd.append(argv); + } - /* Check for truncation, should alloc these strings dynamically instead */ - osafassert((cmd->len > 0) && (cmd->len < sizeof(cmd->cmd))); - TRACE("cmd=%s", cmd->cmd); + (*cmd)->len = (*cmd)->cmd.length(); + TRACE("cmd=%s", (*cmd)->cmd.c_str()); } /** @@ -1184,7 +1191,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_INSTANTIATE - 1]; if (comptype->saAmfCtRelPathInstantiateCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathInstantiateCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathInstantiateCmd, comptype->saAmfCtDefInstantiateCmdArgv, path_prefix, attributes, "saAmfCompInstantiateCmdArgv"); } @@ -1201,7 +1208,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1]; if (comptype->saAmfCtRelPathTerminateCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathTerminateCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathTerminateCmd, comptype->saAmfCtDefTerminateCmdArgv, path_prefix, attributes, "saAmfCompTerminateCmdArgv"); } @@ -1222,7 +1229,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP - 1]; if (comptype->saAmfCtRelPathCleanupCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathCleanupCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathCleanupCmd, comptype->saAmfCtDefCleanupCmdArgv, path_prefix, attributes, "saAmfCompCleanupCmdArgv"); } @@ -1239,7 +1246,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_AMSTART - 1]; if (comptype->saAmfCtRelPathAmStartCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathAmStartCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathAmStartCmd, comptype->saAmfCtDefAmStartCmdArgv, path_prefix, attributes, "saAmfCompAmStartCmdArgv"); comp->is_am_en = true; @@ -1253,7 +1260,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_AMSTOP - 1]; if (comptype->saAmfCtRelPathAmStopCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathAmStopCmd, + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathAmStopCmd, comptype->saAmfCtDefAmStopCmdArgv, path_prefix, attributes, "saAmfCompAmStopCmdArgv"); } @@ -1266,7 +1273,7 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_HC - 1]; if (comptype->osafAmfCtRelPathHcCmd != nullptr) { - init_clc_cli_command(cmd, comptype->osafAmfCtRelPathHcCmd, + init_clc_cli_command(&cmd, comptype->osafAmfCtRelPathHcCmd, comptype->osafAmfCtDefHcCmdArgv, path_prefix, attributes, "osafAmfCompHcCmdArgv"); comp->is_hc_cmd_configured = true; diff --git a/src/amf/common/amf_d2nmsg.h b/src/amf/common/amf_d2nmsg.h index 957d3a326..9122325b3 100644 --- a/src/amf/common/amf_d2nmsg.h +++ b/src/amf/common/amf_d2nmsg.h @@ -166,11 +166,11 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char init_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for - * initialization of component - * Checkpointing - Sent as a one time - * update. - */ + char *init_info; /* ASCII string of information for + * initialization of component + * Checkpointing - Sent as a one time + * update. + */ char init_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT init_time; /* Time interval within which @@ -184,11 +184,11 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char term_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for - * termination of component - * Checkpointing - Sent as a one time - * update. - */ + char *term_info; /* ASCII string of information for + * termination of component + * Checkpointing - Sent as a one time + * update. + */ char term_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT term_time; /* Time interval within which @@ -202,10 +202,10 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char clean_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for - * cleanup of component Checkpointing - * - Sent as a one time update. - */ + char *clean_info; /* ASCII string of information for + * cleanup of component Checkpointing + * - Sent as a one time update. + */ char clean_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT clean_time; /* Time interval within which @@ -219,12 +219,12 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char amstart_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII - * string of information for - * AM start of a component - * Checkpointing - Sent as a one - * time update. - */ + char *amstart_info; /* ASCII + * string of information for + * AM start of a component + * Checkpointing - Sent as a one + * time update. + */ char amstart_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT amstart_time; /* Time interval within which @@ -238,12 +238,12 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char amstop_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII - * string of information for - * AM start of a component. - * Checkpointing - Sent as a one - * time update. - */ + char *amstop_info; /* ASCII + * string of information for + * AM start of a component. + * Checkpointing - Sent as a one + * time update. + */ char amstop_cmd_arg_info[AVSV_MISC_STR_MAX_SIZE]; SaTimeT amstop_time; /* Time interval within which diff --git a/src/amf/common/amf_defs.h b/src/amf/common/amf_defs.h index 3ee5a5aca..cbc4b8261 100644 --- a/src/amf/common/amf_defs.h +++ b/src/amf/common/amf_defs.h @@ -49,6 +49,9 @@ */ #define AVSV_MISC_STR_MAX_SIZE 256 +/* The define for character SPACE*/ +#define AVSV_CHAR_SPACE 0x20 + /* Minimum preferred num of su in 2N, N+M and NWay red model*/ #define AVSV_SG_2N_PREF_INSVC_SU_MIN 2 -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-09-20 10:04:16
|
Hi Tai, Check my comment [Thien] below. Follow google convention in this patch. Best Regards, Thien ________________________________ From: Tai Nguyen <tai...@en...> Sent: Tuesday, September 17, 2024 10:33 AM To: Thien Huynh <thi...@en...>; Thang Nguyen <tha...@en...>; Dat Phan <dat...@en...> Cc: ope...@li... <ope...@li...>; Tai Nguyen <tai...@en...> Subject: [PATCH 1/1] amf: Enhance amf clc cli with dynamic buffer [#3358] With the large size of clc-cli will lead to coredump with buffer overflow The fix is dynamic buffer to amf clc-cli --- src/amf/amfd/comp.cc | 113 +++++++++++++++++----------------- src/amf/amfd/comp.h | 22 +++---- src/amf/amfd/comptype.cc | 33 ++++++---- src/amf/amfnd/avnd_comp.h | 43 +++++++------ src/amf/amfnd/clc.cc | 15 +++-- src/amf/amfnd/compdb.cc | 118 +++++++++++++++++++++++++++--------- src/amf/common/amf_d2nmsg.h | 10 +-- 7 files changed, 218 insertions(+), 136 deletions(-) diff --git a/src/amf/amfd/comp.cc b/src/amf/amfd/comp.cc index 7f83bd9da..7cca09f22 100644 --- a/src/amf/amfd/comp.cc +++ b/src/amf/amfd/comp.cc @@ -44,6 +44,31 @@ AmfDb<std::string, AVD_COMP> *comp_db = nullptr; +/** + * Initializes a single AVD CLC-CLI command for a component. + */ +static void init_avd_clc_cli_command(char **comp_info, + char *comptype_cmd, + char *comptype_defargv, + const SaImmAttrValuesT_2 **attributes, + const char *comptype_argv) { + size_t size; + const char *str; + char *cmd_argv; + size = strlen(comptype_cmd); + size += 1; /*Increase size for SPACE*/ + if ((str = immutil_getStringAttr(attributes, comptype_argv, + 0)) == nullptr) { + str = comptype_defargv; + } + if (str != nullptr) size += strlen(str); + *comp_info = (char *)calloc(size + 1, sizeof(char)); + strcpy(*comp_info, comptype_cmd); + cmd_argv = *comp_info + strlen(comptype_cmd); + *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ + if (str != nullptr) strcpy(cmd_argv, str); +} + void avd_comp_db_add(AVD_COMP *comp) { const std::string comp_name(Amf::to_string(&comp->comp_info.name)); @@ -227,6 +252,11 @@ void avd_comp_delete(AVD_COMP *comp) { su->remove_comp(comp); avd_comptype_remove_comp(comp); comp_db->erase(Amf::to_string(&comp->comp_info.name)); + free(comp->comp_info.init_info); + free(comp->comp_info.term_info); + free(comp->comp_info.clean_info); + free(comp->comp_info.amstart_info); + free(comp->comp_info.amstop_info); delete comp; } @@ -1223,8 +1253,6 @@ static AVD_COMP *comp_create(const std::string &dn, const SaImmAttrValuesT_2 **attributes) { int rc = -1; AVD_COMP *comp; - char *cmd_argv; - const char *str; const AVD_COMP_TYPE *comptype; SaNameT comp_type; SaAisErrorT error; @@ -1259,17 +1287,11 @@ static AVD_COMP *comp_create(const std::string &dn, goto done; } - if (strlen(comptype->saAmfCtRelPathInstantiateCmd) > 0) { - strcpy(comp->comp_info.init_info, comptype->saAmfCtRelPathInstantiateCmd); - cmd_argv = comp->comp_info.init_info + strlen(comp->comp_info.init_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompInstantiateCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefInstantiateCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (comptype->saAmfCtRelPathInstantiateCmd != NULL) { + init_avd_clc_cli_command(&comp->comp_info.init_info, + comptype->saAmfCtRelPathInstantiateCmd, + comptype->saAmfCtDefInstantiateCmdArgv, + attributes, "saAmfCompInstantiateCmdArgv"); comp->comp_info.init_len = strlen(comp->comp_info.init_info); } @@ -1301,17 +1323,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->inst_retry_delay = avd_comp_global_attrs.saAmfDelayBetweenInstantiateAttempts; - if (strlen(comptype->saAmfCtRelPathTerminateCmd) > 0) { - strcpy(comp->comp_info.term_info, comptype->saAmfCtRelPathTerminateCmd); - cmd_argv = comp->comp_info.term_info + strlen(comp->comp_info.term_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompTerminateCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefTerminateCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (comptype->saAmfCtRelPathTerminateCmd != NULL) { + init_avd_clc_cli_command(&comp->comp_info.term_info, + comptype->saAmfCtRelPathTerminateCmd, + comptype->saAmfCtDefTerminateCmdArgv, + attributes, "saAmfCompTerminateCmdArgv"); comp->comp_info.term_len = strlen(comp->comp_info.term_info); } @@ -1321,17 +1337,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->comp_info.terminate_callback_timeout = comptype->saAmfCtDefCallbackTimeout; - if (strlen(comptype->saAmfCtRelPathCleanupCmd) > 0) { - strcpy(comp->comp_info.clean_info, comptype->saAmfCtRelPathCleanupCmd); - cmd_argv = comp->comp_info.clean_info + strlen(comp->comp_info.clean_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompCleanupCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefCleanupCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (comptype->saAmfCtRelPathCleanupCmd != NULL) { + init_avd_clc_cli_command(&comp->comp_info.clean_info, + comptype->saAmfCtRelPathCleanupCmd, + comptype->saAmfCtDefCleanupCmdArgv, + attributes, "saAmfCompCleanupCmdArgv"); comp->comp_info.clean_len = strlen(comp->comp_info.clean_info); } @@ -1339,18 +1349,11 @@ static AVD_COMP *comp_create(const std::string &dn, attributes, 0, &comp->comp_info.clean_time) != SA_AIS_OK) comp->comp_info.clean_time = comptype->saAmfCtDefClcCliTimeout; - if (strlen(comptype->saAmfCtRelPathAmStartCmd) > 0) { - strcpy(comp->comp_info.amstart_info, comptype->saAmfCtRelPathAmStartCmd); - cmd_argv = - comp->comp_info.amstart_info + strlen(comp->comp_info.amstart_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompAmStartCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefAmStartCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); - + if (comptype->saAmfCtRelPathAmStartCmd != NULL) { + init_avd_clc_cli_command(&comp->comp_info.amstart_info, + comptype->saAmfCtRelPathAmStartCmd, + comptype->saAmfCtDefAmStartCmdArgv, + attributes, "saAmfCompAmStartCmdArgv"); comp->comp_info.amstart_len = strlen(comp->comp_info.amstart_info); } @@ -1365,17 +1368,11 @@ static AVD_COMP *comp_create(const std::string &dn, comp->comp_info.max_num_amstart = avd_comp_global_attrs.saAmfNumMaxAmStartAttempts; - if (strlen(comptype->saAmfCtRelPathAmStopCmd) > 0) { - strcpy(comp->comp_info.amstop_info, comptype->saAmfCtRelPathAmStopCmd); - cmd_argv = - comp->comp_info.amstop_info + strlen(comp->comp_info.amstop_info); - *cmd_argv++ = 0x20; /* Insert SPACE between cmd and args */ - - if ((str = immutil_getStringAttr(attributes, "saAmfCompAmStopCmdArgv", - 0)) == nullptr) - str = comptype->saAmfCtDefAmStopCmdArgv; - - if (str != nullptr) strcpy(cmd_argv, str); + if (comptype->saAmfCtRelPathAmStopCmd != NULL) { + init_avd_clc_cli_command(&comp->comp_info.amstop_info, + comptype->saAmfCtRelPathAmStopCmd, + comptype->saAmfCtDefAmStopCmdArgv, + attributes, "saAmfCompAmStopCmdArgv"); } if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCompAmStopTimeout"), diff --git a/src/amf/amfd/comp.h b/src/amf/amfd/comp.h index 4265f16d6..d2d818cd6 100644 --- a/src/amf/amfd/comp.h +++ b/src/amf/amfd/comp.h @@ -185,20 +185,20 @@ class AVD_COMP_TYPE { std::string name{}; SaUint32T saAmfCtCompCategory{}; std::string saAmfCtSwBundle{}; - char saAmfCtDefCmdEnv[AVSV_MISC_STR_MAX_SIZE]{}; + char *saAmfCtDefCmdEnv = NULL; SaTimeT saAmfCtDefClcCliTimeout{}; SaTimeT saAmfCtDefCallbackTimeout{}; - char saAmfCtRelPathInstantiateCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefInstantiateCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; + char *saAmfCtRelPathInstantiateCmd = NULL; + char *saAmfCtDefInstantiateCmdArgv = NULL; SaUint32T saAmfCtDefInstantiationLevel{}; - char saAmfCtRelPathTerminateCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefTerminateCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathCleanupCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefCleanupCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathAmStartCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefAmStartCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtRelPathAmStopCmd[AVSV_MISC_STR_MAX_SIZE]{}; - char saAmfCtDefAmStopCmdArgv[AVSV_MISC_STR_MAX_SIZE]{}; [ Thien ] I suggest use std::string in c++, It will be convenience in manage memory and manipulate string + char *saAmfCtRelPathTerminateCmd = NULL; + char *saAmfCtDefTerminateCmdArgv = NULL; + char *saAmfCtRelPathCleanupCmd = NULL; + char *saAmfCtDefCleanupCmdArgv = NULL; + char *saAmfCtRelPathAmStartCmd = NULL; + char *saAmfCtDefAmStartCmdArgv = NULL; + char *saAmfCtRelPathAmStopCmd = NULL; + char *saAmfCtDefAmStopCmdArgv = NULL; SaTimeT saAmfCtDefQuiescingCompleteTimeout{}; SaAmfRecommendedRecoveryT saAmfCtDefRecoveryOnError{}; SaBoolT saAmfCtDefDisableRestart{}; diff --git a/src/amf/amfd/comptype.cc b/src/amf/amfd/comptype.cc index 48a333e8c..accbad7af 100644 --- a/src/amf/amfd/comptype.cc +++ b/src/amf/amfd/comptype.cc @@ -41,6 +41,17 @@ static void comptype_db_add(AVD_COMP_TYPE *compt) { static void comptype_delete(AVD_COMP_TYPE *avd_comp_type) { osafassert(nullptr == avd_comp_type->list_of_comp); comptype_db->erase(avd_comp_type->name); + free(avd_comp_type->saAmfCtDefCmdEnv); + free(avd_comp_type->saAmfCtRelPathInstantiateCmd); + free(avd_comp_type->saAmfCtDefInstantiateCmdArgv); + free(avd_comp_type->saAmfCtRelPathTerminateCmd); + free(avd_comp_type->saAmfCtDefTerminateCmdArgv); + free(avd_comp_type->saAmfCtRelPathCleanupCmd); + free(avd_comp_type->saAmfCtDefCleanupCmdArgv); + free(avd_comp_type->saAmfCtRelPathAmStartCmd); + free(avd_comp_type->saAmfCtDefAmStartCmdArgv); + free(avd_comp_type->saAmfCtRelPathAmStopCmd); + free(avd_comp_type->saAmfCtDefAmStopCmdArgv); delete avd_comp_type; } @@ -97,7 +108,7 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, compt->saAmfCtSwBundle = Amf::to_string(&ct_sw_bundle); if ((str = immutil_getStringAttr(attributes, "saAmfCtDefCmdEnv", 0)) != nullptr) - strcpy(compt->saAmfCtDefCmdEnv, str); + compt->saAmfCtDefCmdEnv = strdup(str); (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtDefClcCliTimeout"), attributes, 0, &compt->saAmfCtDefClcCliTimeout); (void)immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtDefCallbackTimeout"), @@ -105,10 +116,10 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathInstantiateCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathInstantiateCmd, str); + compt->saAmfCtRelPathInstantiateCmd = strdup(str); if ((str = immutil_getStringAttr(attributes, "saAmfCtDefInstantiateCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefInstantiateCmdArgv, str); + compt->saAmfCtDefInstantiateCmdArgv = strdup(str); (void)immutil_getAttr( const_cast<SaImmAttrNameT>("saAmfCtDefInstantiationLevel"), attributes, 0, @@ -116,28 +127,28 @@ static AVD_COMP_TYPE *comptype_create(const std::string &dn, if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathTerminateCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathTerminateCmd, str); + compt->saAmfCtRelPathTerminateCmd = strdup(str); if ((str = immutil_getStringAttr(attributes, "saAmfCtDefTerminateCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefTerminateCmdArgv, str); + compt->saAmfCtDefTerminateCmdArgv = strdup(str); if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathCleanupCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathCleanupCmd, str); + compt->saAmfCtRelPathCleanupCmd = strdup(str); if ((str = immutil_getStringAttr(attributes, "saAmfCtDefCleanupCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefCleanupCmdArgv, str); + compt->saAmfCtDefCleanupCmdArgv = strdup(str); if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathAmStartCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathAmStartCmd, str); + compt->saAmfCtRelPathAmStartCmd = strdup(str); if ((str = immutil_getStringAttr(attributes, "saAmfCtDefAmStartCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefAmStartCmdArgv, str); + compt->saAmfCtDefAmStartCmdArgv = strdup(str); if ((str = immutil_getStringAttr(attributes, "saAmfCtRelPathAmStopCmd", 0)) != nullptr) - strcpy(compt->saAmfCtRelPathAmStopCmd, str); + compt->saAmfCtRelPathAmStopCmd = strdup(str); if ((str = immutil_getStringAttr(attributes, "saAmfCtDefAmStopCmdArgv", 0)) != nullptr) - strcpy(compt->saAmfCtDefAmStopCmdArgv, str); + compt->saAmfCtDefAmStopCmdArgv = strdup(str); if ((IS_COMP_SAAWARE(compt->saAmfCtCompCategory) || IS_COMP_PROXIED_PI(compt->saAmfCtCompCategory) || diff --git a/src/amf/amfnd/avnd_comp.h b/src/amf/amfnd/avnd_comp.h index c3339ac79..be7ef3bad 100644 --- a/src/amf/amfnd/avnd_comp.h +++ b/src/amf/amfnd/avnd_comp.h @@ -91,7 +91,7 @@ typedef enum avnd_comp_clc_cmd_type { /* clc command parameter definition */ typedef struct avnd_comp_clc_param { - char cmd[SAAMF_CLC_LEN]; /* cmd ascii string */ + char *cmd; /* cmd ascii string */ SaTimeT timeout; /* cmd timeout value */ uint32_t len; /* cmd len */ } AVND_COMP_CLC_CMD_PARAM; @@ -690,32 +690,39 @@ typedef struct avnd_comp_tag { void m_AVND_COMP_OPER_STATE_AVD_SYNC(struct avnd_cb_tag *cb, const AVND_COMP *comp, uint32_t &o_rc); +/*marco count argc the clc cmd string*/ +#define m_AVND_COMP_CLC_COUNT_AGRC(clc_cmd, len, count) \ + { \ + uint32_t i = 0; \ + (count)++; \ + while (i < (len)) \ + { \ + if((clc_cmd)[i++] == 0x20){ \ + (count)++; \ + while ((clc_cmd)[i++] == 0x20); \ + } \ + } \ + if ((clc_cmd)[0] == 0x20) (count)--; \ + if ((clc_cmd)[(len) - 1] == 0x20) (count)--; \ + } /* macro to parse the clc cmd string */ -#define m_AVND_COMP_CLC_STR_PARSE(st, sc, ac, av, tav) \ +#define m_AVND_COMP_CLC_STR_PARSE(sc, av) \ { \ - char str[SAAMF_CLC_LEN], *tok = nullptr; \ - /* copy the str as strtok modifies the original str */ \ - strcpy(str, st); \ - ac = 0; \ - if (nullptr != (tok = strtok(str, " "))) { \ - strncpy(sc, tok, SAAMF_CLC_LEN - 1); \ - av[ac] = sc; \ + char* tok = nullptr; uint32_t ac = 0; \ + if (nullptr != (tok = strtok(sc, " "))) { \ + (av)[ac] = strdup(tok); \ } \ - ac++; \ - while ((nullptr != (tok = strtok(nullptr, " "))) && \ - (ac < (AVND_COMP_CLC_PARAM_MAX + 1))) { \ - if (strlen(tok) > AVND_COMP_CLC_PARAM_SIZE_MAX) break; \ - strcpy(tav[ac], tok); \ - av[ac] = tav[ac]; \ + ac = 1; \ + while ((nullptr != (tok = strtok(nullptr, " ")))) { \ + (av)[ac] = strdup(tok); \ ac++; \ } \ if (nullptr != tok) { \ - sc[0] = (char)(long)nullptr; \ - av[0] = nullptr; \ + (av)[0] = nullptr; \ ac = 0; \ } else \ - av[ac] = nullptr; \ + (av)[ac] = nullptr; \ } /* macros for comp proxy status */ diff --git a/src/amf/amfnd/clc.cc b/src/amf/amfnd/clc.cc index 3c35a916f..95cc998bd 100644 --- a/src/amf/amfnd/clc.cc +++ b/src/amf/amfnd/clc.cc @@ -3107,10 +3107,11 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, AVND_CLC_EVT *clc_evt; AVND_EVT *evt = 0; AVND_COMP_CLC_INFO *clc_info = &comp->clc_info; - char scr[SAAMF_CLC_LEN]; - char *argv[AVND_COMP_CLC_PARAM_MAX + 2]; - char tmp_argv[AVND_COMP_CLC_PARAM_MAX + 2][AVND_COMP_CLC_PARAM_SIZE_MAX]; + char *scr; + scr = strdup(comp->clc_info.cmds[cmd_type - 1].cmd); uint32_t argc = 0, rc = NCSCC_RC_SUCCESS, count = 0; + m_AVND_COMP_CLC_COUNT_AGRC(scr, comp->clc_info.cmds[cmd_type - 1].len, argc); + char *argv[argc + 2]; unsigned int env_counter; unsigned int i; SaStringT env; @@ -3283,8 +3284,7 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, arg.env_arg = env_set; /* tokenize the cmd */ - m_AVND_COMP_CLC_STR_PARSE(clc_info->cmds[cmd_type - 1].cmd, scr, argc, argv, - tmp_argv); + m_AVND_COMP_CLC_STR_PARSE(scr, argv); /* populate the cmd-info */ cmd_info.i_script = argv[0]; @@ -3317,7 +3317,9 @@ uint32_t avnd_comp_clc_cmd_execute(AVND_CB *cb, AVND_COMP *comp, free(env_set[i].value); } free(env_set); - + for(count = 0; count < argc; count++) { + free(argv[count]); + } if (NCSCC_RC_SUCCESS != rc) { TRACE_2("The CLC CLI command execution failed"); /* generate a cmd failure event; it'll be executed asynchronously */ @@ -3350,6 +3352,7 @@ err: if (evt) avnd_evt_destroy(evt); done: + free(scr); TRACE_LEAVE2("%u", rc); return rc; } diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc index 991a69214..00b48b5dc 100644 --- a/src/amf/amfnd/compdb.cc +++ b/src/amf/amfnd/compdb.cc @@ -1120,6 +1120,24 @@ done: TRACE_LEAVE(); return rc; } +/** + * Resize a single CLC-CLI command for a component. + * + * If the lenght of command smaller than new size, It will be allocated + * memory again + * + * @param cmd + */ + +static void resize_clc_cli_command(AVND_COMP_CLC_CMD_PARAM **cmd, + uint16_t newsize) { + char page = newsize/SAAMF_CLC_LEN + 1; + char *newBuffer = new char[SAAMF_CLC_LEN*page + 1](); + std::strcpy(newBuffer, (*cmd)->cmd); + delete[] (*cmd)->cmd; + (*cmd)->cmd = newBuffer; + (*cmd)->len = SAAMF_CLC_LEN*page; +} /** * Initializes a single CLC-CLI command for a component. @@ -1134,36 +1152,58 @@ done: * @param attributes * @param attr_name */ -static void init_clc_cli_command(AVND_COMP_CLC_CMD_PARAM *cmd, +static void init_clc_cli_command(AVND_COMP_CLC_CMD_PARAM **cmd, const char *clc_cmd, char **clc_cmd_argv, const char *path_prefix, const SaImmAttrValuesT_2 **attributes, const char *attr_name) { - char *buf = cmd->cmd; + char *buf = (*cmd)->cmd; int i, j; + u_int32_t sum = 0; const char *argv; // prepend with path prefix if available - if (path_prefix == nullptr) - i = snprintf(buf, sizeof(cmd->cmd), "%s", clc_cmd); - else - i = snprintf(buf, sizeof(cmd->cmd), "%s/%s", path_prefix, clc_cmd); + if (path_prefix == nullptr) { + sum +=strlen(clc_cmd); + if ((*cmd)->len <= sum) { + resize_clc_cli_command(cmd, sum); + buf = (*cmd)->cmd; + } + i = snprintf(buf, (*cmd)->len, "%s", clc_cmd); + } + else { + sum +=strlen(path_prefix) + strlen(clc_cmd) + 1; + if ((*cmd)->len <= sum) { + resize_clc_cli_command(cmd, sum); + buf = (*cmd)->cmd; + } + i = snprintf(buf, (*cmd)->len, "%s/%s", path_prefix, clc_cmd); + } // append argv from comp type j = 0; - while ((argv = clc_cmd_argv[j++]) != nullptr) - i += snprintf(&buf[i], sizeof(cmd->cmd) - i, " %s", argv); - + while ((argv = clc_cmd_argv[j++]) != nullptr) { + sum+= strlen(argv) + 1; + if ((*cmd)->len <= sum) { + resize_clc_cli_command(cmd, sum); + buf = (*cmd)->cmd; + } + i += snprintf(&buf[i], (*cmd)->len, " %s", argv); + } // append argv from comp instance j = 0; - while ((argv = immutil_getStringAttr(attributes, attr_name, j++)) != nullptr) - i += snprintf(&buf[i], sizeof(cmd->cmd) - i, " %s", argv); - - cmd->len = i; + while ((argv = immutil_getStringAttr(attributes, attr_name, + j++)) != nullptr) { + sum+= strlen(argv); + if ((*cmd)->len <= sum) { + resize_clc_cli_command(cmd, sum); + buf = (*cmd)->cmd; + } + i += snprintf(&buf[i], (*cmd)->len, " %s", argv); + } + (*cmd)->len = i; - /* Check for truncation, should alloc these strings dynamically instead */ - osafassert((cmd->len > 0) && (cmd->len < sizeof(cmd->cmd))); - TRACE("cmd=%s", cmd->cmd); + TRACE("cmd=%s", (*cmd)->cmd); } /** @@ -1184,11 +1224,14 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_INSTANTIATE - 1]; if (comptype->saAmfCtRelPathInstantiateCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathInstantiateCmd, + /*Need to delete in case reinit attribute*/ + if (cmd->cmd != nullptr) delete[] cmd->cmd; + cmd->cmd = new char[SAAMF_CLC_LEN + 1](); + cmd->len = SAAMF_CLC_LEN; + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathInstantiateCmd, comptype->saAmfCtDefInstantiateCmdArgv, path_prefix, attributes, "saAmfCompInstantiateCmdArgv"); } - if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCompInstantiateTimeout"), attributes, 0, &cmd->timeout) != SA_AIS_OK) { cmd->timeout = comptype->saAmfCtDefClcCliTimeout; @@ -1201,7 +1244,10 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_TERMINATE - 1]; if (comptype->saAmfCtRelPathTerminateCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathTerminateCmd, + if (cmd->cmd != nullptr) delete[] cmd->cmd; + cmd->cmd = new char[SAAMF_CLC_LEN + 1](); + cmd->len = SAAMF_CLC_LEN; + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathTerminateCmd, comptype->saAmfCtDefTerminateCmdArgv, path_prefix, attributes, "saAmfCompTerminateCmdArgv"); } @@ -1222,7 +1268,10 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_CLEANUP - 1]; if (comptype->saAmfCtRelPathCleanupCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathCleanupCmd, + if (cmd->cmd != nullptr) delete[] cmd->cmd; + cmd->cmd = new char[SAAMF_CLC_LEN + 1](); + cmd->len = SAAMF_CLC_LEN; + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathCleanupCmd, comptype->saAmfCtDefCleanupCmdArgv, path_prefix, attributes, "saAmfCompCleanupCmdArgv"); } @@ -1239,12 +1288,14 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_AMSTART - 1]; if (comptype->saAmfCtRelPathAmStartCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathAmStartCmd, + if (cmd->cmd != nullptr) delete[] cmd->cmd; + cmd->cmd = new char[SAAMF_CLC_LEN + 1](); + cmd->len = SAAMF_CLC_LEN; + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathAmStartCmd, comptype->saAmfCtDefAmStartCmdArgv, path_prefix, attributes, "saAmfCompAmStartCmdArgv"); comp->is_am_en = true; } - if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCompAmStartTimeout"), attributes, 0, &cmd->timeout) != SA_AIS_OK) { cmd->timeout = comptype->saAmfCtDefClcCliTimeout; @@ -1253,11 +1304,13 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_AMSTOP - 1]; if (comptype->saAmfCtRelPathAmStopCmd != nullptr) { - init_clc_cli_command(cmd, comptype->saAmfCtRelPathAmStopCmd, + if (cmd->cmd != nullptr) delete[] cmd->cmd; + cmd->cmd = new char[SAAMF_CLC_LEN + 1](); + cmd->len = SAAMF_CLC_LEN; + init_clc_cli_command(&cmd, comptype->saAmfCtRelPathAmStopCmd, comptype->saAmfCtDefAmStopCmdArgv, path_prefix, attributes, "saAmfCompAmStopCmdArgv"); } - if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCompAmStopTimeout"), attributes, 0, &cmd->timeout) != SA_AIS_OK) { cmd->timeout = comptype->saAmfCtDefClcCliTimeout; @@ -1266,15 +1319,16 @@ static void init_clc_cli_attributes(AVND_COMP *comp, cmd = &comp->clc_info.cmds[AVND_COMP_CLC_CMD_TYPE_HC - 1]; if (comptype->osafAmfCtRelPathHcCmd != nullptr) { - init_clc_cli_command(cmd, comptype->osafAmfCtRelPathHcCmd, + if (cmd->cmd != nullptr) delete[] cmd->cmd; + cmd->cmd = new char[SAAMF_CLC_LEN + 1](); + cmd->len = SAAMF_CLC_LEN; + init_clc_cli_command(&cmd, comptype->osafAmfCtRelPathHcCmd, comptype->osafAmfCtDefHcCmdArgv, path_prefix, attributes, "osafAmfCompHcCmdArgv"); comp->is_hc_cmd_configured = true; } - TRACE_LEAVE(); } - /** * Initialize the members of the comp object with the configuration attributes * from IMM. @@ -1487,6 +1541,8 @@ done1: */ void avnd_comp_delete(AVND_COMP *comp) { SaStringT env; + char *cmd; + int cmd_counter = 0; /* Free saAmfCompCmdEnv[i] before freeing saAmfCompCmdEnv */ if (comp->saAmfCompCmdEnv != nullptr) { @@ -1495,6 +1551,14 @@ void avnd_comp_delete(AVND_COMP *comp) { delete[] env; delete[] comp->saAmfCompCmdEnv; } + /*Free these CLC-CLI command */ + while (cmd_counter < (AVND_COMP_CLC_CMD_TYPE_MAX - 1)) + { + if ((cmd = comp->clc_info.cmds[cmd_counter++].cmd) != nullptr) + { + delete[] cmd; + } + } delete comp->use_comptype_attr; delete comp; diff --git a/src/amf/common/amf_d2nmsg.h b/src/amf/common/amf_d2nmsg.h index 957d3a326..b53dc1051 100644 --- a/src/amf/common/amf_d2nmsg.h +++ b/src/amf/common/amf_d2nmsg.h @@ -166,7 +166,7 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char init_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for + char *init_info; /* ASCII string of information for * initialization of component * Checkpointing - Sent as a one time * update. @@ -184,7 +184,7 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char term_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for + char *term_info; /* ASCII string of information for * termination of component * Checkpointing - Sent as a one time * update. @@ -202,7 +202,7 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char clean_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII string of information for + char *clean_info; /* ASCII string of information for * cleanup of component Checkpointing * - Sent as a one time update. */ @@ -219,7 +219,7 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char amstart_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII + char *amstart_info; /* ASCII * string of information for * AM start of a component * Checkpointing - Sent as a one @@ -238,7 +238,7 @@ typedef struct avsv_comp_info_tag { * Checkpointing - Sent as a one time update. */ - char amstop_info[AVSV_MISC_STR_MAX_SIZE]; /* ASCII + char *amstop_info; /* ASCII * string of information for * AM start of a component. * Checkpointing - Sent as a one -- 2.25.1 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-08-30 05:20:11
|
In case finalize get the head pointer before mutex lock. That leads to the iterator operation corrupt. This fix is to take a mutex lock before get head pointer --- src/ntf/agent/ntfa_api.c | 5 +++++ src/ntf/agent/ntfa_util.c | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ntf/agent/ntfa_api.c b/src/ntf/agent/ntfa_api.c index e89479bf6..79f55714c 100644 --- a/src/ntf/agent/ntfa_api.c +++ b/src/ntf/agent/ntfa_api.c @@ -1799,16 +1799,21 @@ SaAisErrorT saNtfFinalize(SaNtfHandleT ntfHandle) ** including all resources allocated by client if MDS send is ** succesful. **/ + ncshm_give_hdl(ntfHandle); + pthread_mutex_lock(&ntfa_cb.cb_lock); rc = ntfa_hdl_rec_del(&ntfa_cb.client_list, hdl_rec); + pthread_mutex_unlock(&ntfa_cb.cb_lock); if (rc != NCSCC_RC_SUCCESS) { TRACE_1("ntfa_hdl_rec_del failed"); rc = SA_AIS_ERR_BAD_HANDLE; } + goto done_shutdown; } done_give_hdl: ncshm_give_hdl(ntfHandle); +done_shutdown: if (rc == SA_AIS_OK) { rc = ntfa_shutdown(false); if (rc != NCSCC_RC_SUCCESS) diff --git a/src/ntf/agent/ntfa_util.c b/src/ntf/agent/ntfa_util.c index 379348ab5..4d0496d20 100644 --- a/src/ntf/agent/ntfa_util.c +++ b/src/ntf/agent/ntfa_util.c @@ -1102,9 +1102,7 @@ uint32_t ntfa_hdl_rec_del(ntfa_client_hdl_rec_t **list_head, /* Remove subscribers of this client if there are any in subcriberNoList */ - pthread_mutex_lock(&ntfa_cb.cb_lock); ntfa_subscriber_del_by_handle(rm_node->local_hdl); - pthread_mutex_unlock(&ntfa_cb.cb_lock); /* If the to be removed record is the first record */ if (list_iter == rm_node) { -- 2.43.0 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-08-30 05:18:20
|
--- src/ntf/apitest/tet_saNtfFinalize.cc | 63 ++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/ntf/apitest/tet_saNtfFinalize.cc b/src/ntf/apitest/tet_saNtfFinalize.cc index adf2167a9..7e9b117b7 100644 --- a/src/ntf/apitest/tet_saNtfFinalize.cc +++ b/src/ntf/apitest/tet_saNtfFinalize.cc @@ -16,11 +16,14 @@ */ #include <unistd.h> #include <pthread.h> +#include <sys/syscall.h> +#include <sys/wait.h> #include "osaf/apitest/utest.h" #include "osaf/apitest/util.h" #include "ntf/apitest/tet_ntf.h" #include "ntf/apitest/ntf_api_with_try_again.h" +#include "tet_ntf_common.h" SaNtfStateChangeNotificationT myNotification; void saNtfFinalize_01(void) { @@ -156,6 +159,65 @@ void saNtfFinalize_06(void) { test_validate(rc, SA_AIS_ERR_BAD_HANDLE); } +void randomSleep() { + srand(time(0)); + const timespec delay{rand() % 10, 0}; + base::Sleep(delay); +} + +void *initializeAndFinalize(void *arg) { + pid_t tid = syscall(SYS_gettid); + SaNtfHandleT ntfHandle1 = 0; + SaAisErrorT rc = SA_AIS_OK; + + randomSleep(); + while ((rc = NtfTest::saNtfInitialize(&ntfHandle1, &ntfSendCallbacks, + &ntfVersion)) != SA_AIS_OK) { + base::Sleep(base::kOneSecond); + printf("thread[%d]: init failed with error %s\n", tid, get_saf_error(rc)); + } + + randomSleep(); + if ((rc = NtfTest::saNtfFinalize(ntfHandle1)) != SA_AIS_OK) { + printf("thread[%d]: finalize failed with error %s\n", tid, + get_saf_error(rc)); + } + + pthread_exit(NULL); +} + +void saNtfFinalize_07(void) { + pid_t pid = fork(); + if (pid == -1) { + test_validate(SA_AIS_ERR_LIBRARY, SA_AIS_OK); + } else if (pid == 0) { + // Child process + size_t clientmax = 500; + pthread_t thread[clientmax]; + for (size_t i = 0; i < clientmax; i++) { + pthread_create(&thread[i], NULL, initializeAndFinalize, nullptr); + } + for (size_t i = 0; i < clientmax; i++) { + pthread_join(thread[i], NULL); + } + exit(0); + } else { + // Parent process + int status; + wait(&status); + if (WIFEXITED(status)) { + printf("Child process exited, status=%d\n", WEXITSTATUS(status)); + test_validate(WEXITSTATUS(status), 0); + } else if (WIFSIGNALED(status) && WCOREDUMP(status)) { + printf("Coredump in child process\n"); + test_validate(SA_AIS_ERR_LIBRARY, SA_AIS_OK); + } else { + printf("Child process was stopped by a signal\n"); + test_validate(SA_AIS_OK, SA_AIS_OK); + } + } +} + __attribute__((constructor)) static void saNtfFinalize_constructor(void) { test_suite_add(2, "Life cycle, finalize, API 2"); test_case_add(2, saNtfFinalize_01, "saNtfFinalize SA_AIS_OK"); @@ -170,4 +232,5 @@ __attribute__((constructor)) static void saNtfFinalize_constructor(void) { "SA_AIS_OK"); test_case_add(2, saNtfFinalize_06, "saNtfFinalize SA_AIS_ERR_BAD_HANDLE - unintilized handle"); + test_case_add(2, saNtfFinalize_07, "Intialize and saNtfFinalize in parallel"); } -- 2.43.0 The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |
From: Thien H. <thi...@en...> - 2024-08-30 05:00:32
|
Summary: ntf: fix thread unsafe [#3357] Review request for Ticket(s): 3357 Peer Reviewer(s): Thang, Dat Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3357 Base revision: 39f81573f1ed7fed69661bdbed188ebcbedaad7d Personal repository: git://git.code.sf.net/u/thienhuynh/review -------------------------------- Impacted area Impact y/n -------------------------------- Docs n Build system n RPM/packaging n Configuration files n Startup scripts n SAF services n OpenSAF services n Core libraries y Samples n Tests n Other n Comments (indicate scope for each "y" above): --------------------------------------------- *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision 7edd13a68302344a5e552e9e28c04c7a882fe684 Author: thien.m.huynh <thi...@en...> Date: Wed, 14 Aug 2024 16:27:44 +0700 ntf: test case to test manage ntfa resource [#3357] revision 862493e85ee889bbb0d2369e357d9360214bc39f Author: thien.m.huynh <thi...@en...> Date: Wed, 14 Aug 2024 16:00:05 +0700 ntf: fix thread unsafe [#3357] In case finalize get the head pointer before mutex lock. That leads to the iterator operation corrupt. This fix is to take a mutex lock before get head pointer Complete diffstat: ------------------ src/ntf/agent/ntfa_api.c | 5 +++ src/ntf/agent/ntfa_util.c | 2 -- src/ntf/apitest/tet_saNtfFinalize.cc | 63 ++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) Testing Commands: ----------------- N/A Testing, Expected Results: -------------------------- N/A Conditions of Submission: ------------------------- ACK from reviewer Arch Built Started Linux distro ------------------------------------------- mips n n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: ------------------- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection. Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions. |