> -----Original Message----- > From: Hans Nordebäck > Sent: den 15 januari 2014 12:27 > To: Hans Feldt > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 1 of 1] amfd: use stl::map in app [#713] > > ack with one comment, why use a pointer ,static AmfAppMap *db instead of > static AmfAppMap db, then new (delete) is not needed?/BR HansN
Because according to google rules: " Static or global variables of class type are forbidden: they cause hard-to-find bugs due to indeterminate order of construction and destruction. " Isn't that a correct interpretation? /HansF > On 01/15/14 09:11, Hans Feldt wrote: > > osaf/services/saf/amf/amfd/app.cc | 95 > > ++++++++++++++----------------- > > osaf/services/saf/amf/amfd/ckpt_enc.cc | 11 +-- > > osaf/services/saf/amf/amfd/ckpt_updt.cc | 7 +- > > osaf/services/saf/amf/amfd/include/app.h | 21 +++++-- > > osaf/services/saf/amf/amfd/sg.cc | 2 +- > > osaf/services/saf/amf/amfd/si.cc | 2 +- > > osaf/services/saf/amf/amfd/util.cc | 16 ++-- > > 7 files changed, 74 insertions(+), 80 deletions(-) > > > > > > diff --git a/osaf/services/saf/amf/amfd/app.cc > > b/osaf/services/saf/amf/amfd/app.cc > > --- a/osaf/services/saf/amf/amfd/app.cc > > +++ b/osaf/services/saf/amf/amfd/app.cc > > @@ -24,60 +24,47 @@ > > #include <imm.h> > > #include <si.h> > > > > -static NCS_PATRICIA_TREE app_db; > > +// A map with application instances, key is DN as std:string > > +AmfAppMap *AmfAppDb::db; > > > > -void avd_app_db_add(AVD_APP *app) > > -{ > > - unsigned int rc; > > - > > - if (avd_app_get(&app->name) == NULL) { > > - rc = ncs_patricia_tree_add(&app_db, &app->tree_node); > > - osafassert(rc == NCSCC_RC_SUCCESS); > > - } > > +void AmfAppDb::insert(AVD_APP *app) { > > + std::string name((const char*)app->name.value, app->name.length); > > + (*db)[name] = app; > > } > > > > -AVD_APP *avd_app_new(const SaNameT *dn) > > +void AmfAppDb::erase(AVD_APP *app) { > > + std::string name((const char*)app->name.value, app->name.length); > > + AmfAppMap::iterator it = db->find(name); > > + db->erase(it); > > +} > > + > > +AVD_APP *AmfAppDb::find(const SaNameT *dn) { > > + std::string name((const char*)dn->value, dn->length); > > + AmfAppMap::iterator it = db->find(name); > > + if (it == db->end()) > > + return NULL; > > + else > > + return it->second; > > +} > > + > > +// TODO(hafe) change this to a constructor > > +static AVD_APP *avd_app_new(const SaNameT *dn) > > { > > - AVD_APP *app; > > - > > - app = new AVD_APP(); > > - > > + AVD_APP *app = new AVD_APP(); > > memcpy(app->name.value, dn->value, dn->length); > > app->name.length = dn->length; > > - app->tree_node.key_info = (uint8_t *)&(app->name); > > - > > return app; > > } > > > > -void avd_app_delete(AVD_APP *app) > > +// TODO(hafe) change this to a destructor > > +static void avd_app_delete(AVD_APP *app) > > { > > - unsigned int rc = ncs_patricia_tree_del(&app_db, &app->tree_node); > > - osafassert(rc == NCSCC_RC_SUCCESS); > > + AmfAppDb::erase(app); > > m_AVSV_SEND_CKPT_UPDT_ASYNC_RMV(avd_cb, app, AVSV_CKPT_AVD_APP_CONFIG); > > avd_apptype_remove_app(app); > > delete app; > > } > > > > -AVD_APP *avd_app_get(const SaNameT *dn) > > -{ > > - SaNameT tmp = {0}; > > - > > - tmp.length = dn->length; > > - memcpy(tmp.value, dn->value, tmp.length); > > - > > - return (AVD_APP *)ncs_patricia_tree_get(&app_db, (uint8_t *)&tmp); > > -} > > - > > -AVD_APP *avd_app_getnext(const SaNameT *dn) > > -{ > > - SaNameT tmp = {0}; > > - > > - tmp.length = dn->length; > > - memcpy(tmp.value, dn->value, tmp.length); > > - > > - return (AVD_APP *)ncs_patricia_tree_getnext(&app_db, (uint8_t *)&tmp); > > -} > > - > > static void app_add_to_model(AVD_APP *app) > > { > > TRACE_ENTER2("%s", app->name.value); > > @@ -88,7 +75,10 @@ static void app_add_to_model(AVD_APP *ap > > goto done; > > } > > > > - avd_app_db_add(app); > > + if (AmfAppDb::find(&app->name) == NULL) { > > + AmfAppDb::insert(app); > > + } > > + > > /* Find application type and make a link with app type */ > > app->app_type = avd_apptype_get(&app->saAmfAppType); > > osafassert(app->app_type); > > @@ -229,7 +219,8 @@ AVD_APP *avd_app_create(const SaNameT *d > > ** If called at new active at failover, the object is found in the DB > > ** but needs to get configuration attributes initialized. > > */ > > - if (NULL == (app = avd_app_get(dn))) { > > + app = AmfAppDb::find(dn); > > + if (app == NULL) { > > if ((app = avd_app_new(dn)) == NULL) > > goto done; > > } else > > @@ -314,7 +305,7 @@ static void app_ccb_apply_cb(CcbUtilOper > > break; > > case CCBUTIL_MODIFY: { > > const SaImmAttrModificationT_2 *attr_mod; > > - app = avd_app_get(&opdata->objectName); > > + app = AmfAppDb::find(&opdata->objectName); > > > > while ((attr_mod = opdata->param.modify.attrMods[i++]) != NULL) > > { > > const SaImmAttrValuesT_2 *attribute = > > &attr_mod->modAttr; > > @@ -334,7 +325,7 @@ static void app_ccb_apply_cb(CcbUtilOper > > break; > > } > > case CCBUTIL_DELETE: > > - app = avd_app_get(&opdata->objectName); > > + app = AmfAppDb::find(&opdata->objectName); > > /* by this time all the SGs and SIs under this > > * app object should have been *DELETED* just > > * do a sanity check here > > @@ -359,7 +350,7 @@ static void app_admin_op_cb(SaImmOiHandl > > TRACE_ENTER2("%s", object_name->value); > > > > /* Find the app name. */ > > - app = avd_app_get(object_name); > > + app = AmfAppDb::find(object_name); > > osafassert(app != NULL); > > > > if (op_id == SA_AMF_ADMIN_UNLOCK) { > > @@ -416,7 +407,7 @@ done: > > static SaAisErrorT app_rt_attr_cb(SaImmOiHandleT immOiHandle, > > const SaNameT *objectName, const SaImmAttrNameT *attributeNames) > > { > > - AVD_APP *app = avd_app_get(objectName); > > + AVD_APP *app = AmfAppDb::find(objectName); > > SaImmAttrNameT attributeName; > > int i = 0; > > > > @@ -493,12 +484,10 @@ SaAisErrorT avd_app_config_get(void) > > > > void avd_app_constructor(void) > > { > > - NCS_PATRICIA_PARAMS patricia_params; > > - > > - patricia_params.key_size = sizeof(SaNameT); > > - osafassert(ncs_patricia_tree_init(&app_db, &patricia_params) == > > NCSCC_RC_SUCCESS); > > - > > - avd_class_impl_set(const_cast<SaImmClassNameT>("SaAmfApplication"), > > app_rt_attr_cb, app_admin_op_cb, > > - app_ccb_completed_cb, app_ccb_apply_cb); > > + AmfAppDb::db = new AmfAppMap; > > + avd_class_impl_set(const_cast<SaImmClassNameT>("SaAmfApplication"), > > + app_rt_attr_cb, > > + app_admin_op_cb, > > + app_ccb_completed_cb, > > + app_ccb_apply_cb); > > } > > - > > diff --git a/osaf/services/saf/amf/amfd/ckpt_enc.cc > > b/osaf/services/saf/amf/amfd/ckpt_enc.cc > > --- a/osaf/services/saf/amf/amfd/ckpt_enc.cc > > +++ b/osaf/services/saf/amf/amfd/ckpt_enc.cc > > @@ -2243,16 +2243,12 @@ static uint32_t enc_cs_node_config(AVD_C > > static uint32_t enc_cs_app_config(AVD_CL_CB *cb, NCS_MBCSV_CB_ENC *enc, > > uint32_t *num_of_obj) > > { > > uint32_t status = NCSCC_RC_SUCCESS; > > - SaNameT app_name; > > - AVD_APP *app; > > EDU_ERR ederror = static_cast<EDU_ERR>(0); > > TRACE_ENTER(); > > > > - /* > > - * Walk through the entire list and send the entire list data. > > - */ > > - app_name.length = 0; > > - for (app = avd_app_getnext(&app_name); app != NULL; app = > > avd_app_getnext(&app_name)) { > > + /* Walk through all application instances and encode. */ > > + for (AmfAppMap::iterator it = AmfAppDb::db->begin(); it != > > AmfAppDb::db->end(); it++) { > > + AVD_APP *app = it->second; > > status = m_NCS_EDU_VER_EXEC(&cb->edu_hdl, > > avsv_edp_ckpt_msg_app, &enc->io_uba, > > EDP_OP_TYPE_ENC, app, &ederror, > > enc->i_peer_version); > > > > @@ -2261,7 +2257,6 @@ static uint32_t enc_cs_app_config(AVD_CL > > return NCSCC_RC_FAILURE; > > } > > > > - app_name = app->name; > > (*num_of_obj)++; > > } > > > > diff --git a/osaf/services/saf/amf/amfd/ckpt_updt.cc > > b/osaf/services/saf/amf/amfd/ckpt_updt.cc > > --- a/osaf/services/saf/amf/amfd/ckpt_updt.cc > > +++ b/osaf/services/saf/amf/amfd/ckpt_updt.cc > > @@ -18,6 +18,7 @@ > > #include <logtrace.h> > > #include <amfd.h> > > #include <csi.h> > > +#include <app.h> > > > > static char *action_name[] = { > > const_cast<char*>("invalid"), > > @@ -95,14 +96,14 @@ done: > > uint32_t avd_ckpt_app(AVD_CL_CB *cb, AVD_APP *ckpt_app, > > NCS_MBCSV_ACT_TYPE action) > > { > > uint32_t rc = NCSCC_RC_SUCCESS; > > - AVD_APP *app; > > > > TRACE_ENTER2("%s - '%s'", action_name[action], ckpt_app->name.value); > > > > osafassert (action == NCS_MBCSV_ACT_UPDATE); > > > > - if (NULL == (app = avd_app_get(&ckpt_app->name))) { > > - LOG_ER("avd_app_get FAILED for '%s'", ckpt_app->name.value); > > + AVD_APP *app = AmfAppDb::find(&ckpt_app->name); > > + if (app == NULL) { > > + LOG_ER("%s failed to find '%s'", __FUNCTION__, > > ckpt_app->name.value); > > rc = NCSCC_RC_FAILURE; > > goto done; > > } > > diff --git a/osaf/services/saf/amf/amfd/include/app.h > > b/osaf/services/saf/amf/amfd/include/app.h > > --- a/osaf/services/saf/amf/amfd/include/app.h > > +++ b/osaf/services/saf/amf/amfd/include/app.h > > @@ -24,8 +24,13 @@ > > #ifndef AVD_APP_H > > #define AVD_APP_H > > > > +#include <map> > > +#include <string> > > + > > #include <saAmf.h> > > #include <saImm.h> > > + > > +// TODO(hafe) remove include below when apptype is using map > > #include <ncspatricia.h> > > #include <sg.h> > > #include <si.h> > > @@ -42,8 +47,8 @@ typedef struct avd_app_type_tag { > > struct avd_app_tag *list_of_app; > > } AVD_APP_TYPE; > > > > +// TODO (hafe) change to class AmfApp > > typedef struct avd_app_tag { > > - NCS_PATRICIA_NODE tree_node; /* key is name */ > > SaNameT name; > > SaNameT saAmfAppType; > > SaAmfAdminStateT saAmfApplicationAdminState; > > @@ -54,11 +59,15 @@ typedef struct avd_app_tag { > > struct avd_app_type_tag *app_type; > > } AVD_APP; > > > > -extern void avd_app_db_add(AVD_APP *app); > > -extern AVD_APP *avd_app_new(const SaNameT *dn); > > -extern void avd_app_delete(AVD_APP *app); > > -extern AVD_APP *avd_app_get(const SaNameT *app_name); > > -extern AVD_APP *avd_app_getnext(const SaNameT *app_name); > > +typedef std::map<std::string, AVD_APP*> AmfAppMap; > > + > > +class AmfAppDb { > > + public: > > + static AmfAppMap *db; > > + static void insert(AVD_APP *app); > > + static void erase(AVD_APP *app); > > + static AVD_APP *find(const SaNameT *name); > > +}; > > > > extern void avd_app_add_si(AVD_APP *app, struct avd_si_tag *si); > > extern void avd_app_remove_si(AVD_APP *app, struct avd_si_tag *si); > > 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 > > @@ -66,7 +66,7 @@ static void sg_add_to_model(AVD_SG *sg) > > } > > > > avsv_sanamet_init(&sg->name, &dn, "safApp"); > > - sg->app = avd_app_get(&dn); > > + sg->app = AmfAppDb::find(&dn); > > > > avd_sg_db_add(sg); > > sg->sg_type = avd_sgtype_get(&sg->saAmfSGType); > > diff --git a/osaf/services/saf/amf/amfd/si.cc > > b/osaf/services/saf/amf/amfd/si.cc > > --- a/osaf/services/saf/amf/amfd/si.cc > > +++ b/osaf/services/saf/amf/amfd/si.cc > > @@ -450,7 +450,7 @@ static void si_add_to_model(AVD_SI *si) > > } > > > > avsv_sanamet_init(&si->name, &dn, "safApp"); > > - si->app = avd_app_get(&dn); > > + si->app = AmfAppDb::find(&dn); > > > > avd_si_db_add(si); > > > > diff --git a/osaf/services/saf/amf/amfd/util.cc > > b/osaf/services/saf/amf/amfd/util.cc > > --- a/osaf/services/saf/amf/amfd/util.cc > > +++ b/osaf/services/saf/amf/amfd/util.cc > > @@ -1391,13 +1391,14 @@ void avsv_sanamet_init_from_association_ > > } > > > > /** > > - * Call this function from GDB to dump the AVD state. Example > > - * below. > > + * Dumps the director state to file > > + * This can be done using GDB: > > + * $ gdb --pid=`pgrep osafamfd` -ex 'call > > amfd_file_dump("/tmp/osafamfd.dump")' \ > > + * /usr/local/lib/opensaf/osafamfd > > * > > * @param path > > */ > > -/* gdb --pid=`pgrep ncs_scap` -ex 'call avd_file_dump("/tmp/avd.dump")' > > /usr/local/lib/opensaf/ncs_scap */ > > -void avd_file_dump(const char *path) > > +void amfd_file_dump(const char *path) > > { > > SaNameT dn = {0}; > > FILE *f = fopen(path, "w"); > > @@ -1422,13 +1423,12 @@ void avd_file_dump(const char *path) > > node_id = node->node_info.nodeId; > > } > > > > - AVD_APP *app; > > - dn.length = 0; > > - for (app = avd_app_getnext(&dn); app != NULL; app = > > avd_app_getnext(&dn)) { > > + for (AmfAppMap::iterator it = AmfAppDb::db->begin(); > > + it != AmfAppDb::db->end(); it++) { > > + const AVD_APP *app = it->second; > > fprintf(f, "%s\n", app->name.value); > > fprintf(f, "\tsaAmfApplicationAdminState=%u\n", > > app->saAmfApplicationAdminState); > > fprintf(f, "\tsaAmfApplicationCurrNumSGs=%u\n", > > app->saAmfApplicationCurrNumSGs); > > - dn = app->name; > > } > > > > AVD_SG *sg; ------------------------------------------------------------------------------ CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments & Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel