But isn't it better to implement the db class as a singleton then? class AmfAppDb { public: static AmfAppDb* get_instance(); or static AmfAppDb& get_instance();
void insert(AVD_APP *app); void erase(AVD_APP *app); AVD_APP *find(const SaNameT *name); private: AmfAppMap db; }; /BR Hans On 01/15/14 13:56, Hans Feldt wrote: >> -----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