> -----Original Message-----
> From: Hans Nordebäck
> Sent: den 15 januari 2014 12:27
> To: Hans Feldt
> Cc: [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
