Hi Nagu, please see comment inlined with [HansN]/Thanks HansN
On 09/09/2015 11:17 AM, Nagendra Kumar wrote: > Please find comment inlined with [Nagu]. > > Thanks > -Nagu >> -----Original Message----- >> From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] >> Sent: 14 August 2015 20:20 >> To: Praveen Malviya; Nagendra Kumar; gary....@dektech.com.au >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: [PATCH 07 of 10] amfd: Make AVD_AMF_SG_TYPE a class [#1142] >> >> osaf/services/saf/amf/amfd/include/sg.h | 3 +- >> osaf/services/saf/amf/amfd/include/sgtype.h | 39 ++++++++----- >> osaf/services/saf/amf/amfd/sg.cc | 1 - >> osaf/services/saf/amf/amfd/sgtype.cc | 84 >> +++++++++++----------------- >> 4 files changed, 59 insertions(+), 68 deletions(-) >> >> >> diff --git a/osaf/services/saf/amf/amfd/include/sg.h >> b/osaf/services/saf/amf/amfd/include/sg.h >> --- a/osaf/services/saf/amf/amfd/include/sg.h >> +++ b/osaf/services/saf/amf/amfd/include/sg.h >> @@ -192,8 +192,7 @@ public: >> SaInvocationT adminOp_invocationId; >> SaAmfAdminOperationIdT adminOp; >> >> - AVD_SG *sg_list_sg_type_next; >> - struct avd_amf_sg_type_tag *sg_type; >> + AVD_AMF_SG_TYPE *sg_type; >> AVD_SG *sg_list_app_next; >> AVD_APP *app; >> bool equal_ranked_su; /* This flag is set when ranks of all SU is the >> same. >> diff --git a/osaf/services/saf/amf/amfd/include/sgtype.h >> b/osaf/services/saf/amf/amfd/include/sgtype.h >> --- a/osaf/services/saf/amf/amfd/include/sgtype.h >> +++ b/osaf/services/saf/amf/amfd/include/sgtype.h >> @@ -27,28 +27,37 @@ >> >> #include <saAmf.h> >> #include <include/db_template.h> >> +#include <vector> >> >> class AVD_SG; >> >> -typedef struct avd_amf_sg_type_tag { >> - SaNameT name; >> - bool saAmfSgtDefAutoRepair_configured; /* True when user >> configures saAmfSGDefAutoRepair else false */ >> +class AVD_AMF_SG_TYPE { >> + public: >> + explicit AVD_AMF_SG_TYPE(const SaNameT *dn); >> + SaNameT name {}; >> + bool saAmfSgtDefAutoRepair_configured {}; /* True when user configures >> saAmfSGDefAutoRepair else false */ >> /******************** B.04 model >> *************************************************/ >> - SaNameT *saAmfSGtValidSuTypes; /* array of DNs, size in >> number_su_type */ >> - SaAmfRedundancyModelT saAmfSgtRedundancyModel; >> - SaBoolT saAmfSgtDefAutoRepair; >> - SaBoolT saAmfSgtDefAutoAdjust; >> - SaTimeT saAmfSgtDefAutoAdjustProb; >> - SaTimeT saAmfSgtDefCompRestartProb; >> - SaUint32T saAmfSgtDefCompRestartMax; >> - SaTimeT saAmfSgtDefSuRestartProb; >> - SaUint32T saAmfSgtDefSuRestartMax; >> + SaNameT *saAmfSGtValidSuTypes {}; /* array of DNs, size in >> number_su_type */ >> + SaAmfRedundancyModelT saAmfSgtRedundancyModel {}; >> + SaBoolT saAmfSgtDefAutoRepair {}; >> + SaBoolT saAmfSgtDefAutoAdjust {}; >> + SaTimeT saAmfSgtDefAutoAdjustProb {}; >> + SaTimeT saAmfSgtDefCompRestartProb {}; >> + SaUint32T saAmfSgtDefCompRestartMax {}; >> + SaTimeT saAmfSgtDefSuRestartProb {}; >> + SaUint32T saAmfSgtDefSuRestartMax {}; >> /******************** B.04 model >> *************************************************/ >> >> - uint32_t number_su_type; /* size of array saAmfSGtValidSuTypes >> */ >> - AVD_SG *list_of_sg; >> + uint32_t number_su_type {}; /* size of array saAmfSGtValidSuTypes */ >> + std::vector<AVD_SG*> list_of_sg {}; >> + private: >> + AVD_AMF_SG_TYPE(); >> + void initialize(); >> + // disallow copy and assign >> + AVD_AMF_SG_TYPE(const AVD_AMF_SG_TYPE&); >> + void operator=(const AVD_AMF_SG_TYPE&); >> >> -} AVD_AMF_SG_TYPE; >> +}; >> >> extern AmfDb<std::string, AVD_AMF_SG_TYPE> *sgtype_db; >> SaAisErrorT avd_sgtype_config_get(void); >> diff --git a/osaf/services/saf/amf/amfd/sg.cc >> b/osaf/services/saf/amf/amfd/sg.cc >> --- a/osaf/services/saf/amf/amfd/sg.cc >> +++ b/osaf/services/saf/amf/amfd/sg.cc >> @@ -119,7 +119,6 @@ AVD_SG::AVD_SG(): >> sg_redundancy_model(SA_AMF_NO_REDUNDANCY_MODEL), >> list_of_su(NULL), >> list_of_si(NULL), >> - sg_list_sg_type_next(NULL), >> sg_type(NULL), >> sg_list_app_next(NULL), >> app(NULL), >> diff --git a/osaf/services/saf/amf/amfd/sgtype.cc >> b/osaf/services/saf/amf/amfd/sgtype.cc >> --- a/osaf/services/saf/amf/amfd/sgtype.cc >> +++ b/osaf/services/saf/amf/amfd/sgtype.cc >> @@ -26,42 +26,27 @@ >> #include <ntf.h> >> #include <sgtype.h> >> #include <proc.h> >> +#include <algorithm> >> >> AmfDb<std::string, AVD_AMF_SG_TYPE> *sgtype_db = NULL; >> >> void avd_sgtype_add_sg(AVD_SG *sg) >> { >> - sg->sg_list_sg_type_next = sg->sg_type->list_of_sg; >> - sg->sg_type->list_of_sg = sg; >> + sg->sg_type->list_of_sg.push_back(sg); > [Nagu]: Though order doesn't matter here, push_back add in the end. Is this > reason to use push_back? [HansN] list_of_sg is a std::vector, there is no push_front on std::vector. > >> } >> >> -void avd_sgtype_remove_sg(AVD_SG *sg) >> -{ >> - AVD_SG *i_sg = NULL; >> - AVD_SG *prev_sg = NULL; >> +void avd_sgtype_remove_sg(AVD_SG *sg) { >> + AVD_AMF_SG_TYPE *sg_type = sg->sg_type; >> >> - if (sg->sg_type != NULL) { >> - i_sg = sg->sg_type->list_of_sg; >> - >> - while ((i_sg != NULL) && (i_sg != sg)) { >> - prev_sg = i_sg; >> - i_sg = i_sg->sg_list_sg_type_next; >> - } >> - >> - if (i_sg != sg) { >> - /* Log a fatal error */ >> - osafassert(0); >> - } else { >> - if (prev_sg == NULL) { >> - sg->sg_type->list_of_sg = sg- >>> sg_list_sg_type_next; >> - } else { >> - prev_sg->sg_list_sg_type_next = sg- >>> sg_list_sg_type_next; >> - } >> - } >> - >> - sg->sg_list_sg_type_next = NULL; >> - sg->sg_type = NULL; >> - } >> + if (sg_type != nullptr) { >> + auto pos = std::find(sg_type->list_of_sg.begin(), >> sg_type->list_of_sg.end(), >> sg); >> + if(pos != sg_type->list_of_sg.end()) { >> + sg_type->list_of_sg.erase(pos); >> + } else { >> + /* Log a fatal error */ >> + osafassert(0); >> + } >> + } >> } >> >> static void sgtype_delete(AVD_AMF_SG_TYPE *sg_type) >> @@ -167,6 +152,12 @@ static int is_config_valid(const SaNameT >> return 1; >> } >> >> +// >> +AVD_AMF_SG_TYPE::AVD_AMF_SG_TYPE(const SaNameT *dn) { >> + memcpy(&name.value, dn->value, dn->length); >> + name.length = dn->length; >> +} >> + >> /** >> * Create a new SG type object and initialize its attributes from the >> attribute >> list. >> * @param dn >> @@ -184,10 +175,7 @@ static AVD_AMF_SG_TYPE *sgtype_create(Sa >> >> TRACE_ENTER2("'%s'", dn->value); >> >> - sgt = new AVD_AMF_SG_TYPE(); >> - >> - memcpy(sgt->name.value, dn->value, dn->length); >> - sgt->name.length = dn->length; >> + sgt = new AVD_AMF_SG_TYPE(dn); >> >> error = >> immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSgtRedundancyModel" >> ), attributes, 0, &sgt->saAmfSgtRedundancyModel); >> osafassert(error == SA_AIS_OK); >> @@ -354,7 +342,7 @@ static SaAisErrorT sgtype_ccb_completed_ >> { >> SaAisErrorT rc = SA_AIS_ERR_BAD_OPERATION; >> AVD_AMF_SG_TYPE *sgt; >> - AVD_SG *sg; >> + >> bool sg_exist = false; >> CcbUtilOperationData_t *t_opData; >> >> @@ -370,23 +358,18 @@ static SaAisErrorT sgtype_ccb_completed_ >> break; >> case CCBUTIL_DELETE: >> sgt = sgtype_db->find(Amf::to_string(&opdata->objectName)); >> - if (sgt->list_of_sg != NULL) { >> - /* check whether there exists a delete operation for >> - * each of the SG in the sg_type list in the current CCB >> - */ >> - sg = sgt->list_of_sg; >> - while (sg != NULL) { >> - t_opData = >> ccbutil_getCcbOpDataByDN(opdata->ccbId, &sg->name); >> - if ((t_opData == NULL) || (t_opData- >>> operationType != CCBUTIL_DELETE)) { >> - sg_exist = true; >> - break; >> - } >> - sg = sg->sg_list_sg_type_next; >> - } >> - if (sg_exist == true) { >> - report_ccb_validation_error(opdata, "SGs >> exist of this SG type '%s'",sgt->name.value); >> + >> + for (const auto& sg : sgt->list_of_sg) { >> + t_opData = ccbutil_getCcbOpDataByDN(opdata- >>> ccbId, &sg->name); >> + if ((t_opData == NULL) || (t_opData->operationType >> != CCBUTIL_DELETE)) { >> + sg_exist = true; >> + break; >> + } >> + } >> + >> + if (sg_exist == true) { >> + report_ccb_validation_error(opdata, "SGs exist of this >> SG type '%s'",sgt->name.value); >> goto done; >> - } >> } >> opdata->userData = sgt; /* Save for later use in apply >> */ >> rc = SA_AIS_OK; >> @@ -438,7 +421,8 @@ static void sgtype_ccb_apply_modify_hdlr >> >> /* Modify saAmfSGAutoRepair for SGs which had >> inherited saAmfSgtDefAutoRepair.*/ >> if (old_value != sgt->saAmfSgtDefAutoRepair) { >> - for (AVD_SG *sg = sgt->list_of_sg; sg; sg = sg- >>> sg_list_sg_type_next) { >> + >> + for (const auto& sg : sgt->list_of_sg) { >> if (!sg- >>> saAmfSGAutoRepair_configured) { >> sg->saAmfSGAutoRepair = >> static_cast<SaBoolT>(sgt->saAmfSgtDefAutoRepair); >> TRACE("Modified >> saAmfSGAutoRepair is '%u'", sg->saAmfSGAutoRepair); ------------------------------------------------------------------------------ Monitor Your Dynamic Infrastructure at Any Scale With Datadog! Get real-time metrics from all of your servers, apps and tools in one place. SourceForge users - Click here to start your Free Trial of Datadog now! http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140 _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
