I have already acked, but one more comment, the member initialization
in a constructor e.g AVD_SU can be used, (if you are not using e.g. init
function)
AVD_SU::AVD_SU(const SaNameT *dn)
: term_state(false),
su_switch(AVSV_SI_TOGGLE_STABLE),
...
{
}
/Regards Hans
On 05/02/14 13:47, Hans Feldt wrote:
>
>> -----Original Message-----
>> From: Nagendra Kumar [mailto:nagendr...@oracle.com]
>> Sent: den 2 maj 2014 12:31
>> To: Hans Feldt; Praveen Malviya; Hans Nordebäck
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [devel] [PATCH 4 of 7] amfd: add SU constr/destr [#713]
>>
>> 1. Can we move all the initialization in default constructor and fill dn
>> name after words.
> [Hans] guess we can do many things, this works. In a later patch I have added
> an init() method. So new+init it will replace the current su_create() function
>> 2. Other pointers are not necessary to initialize, any specific reason for
>> doing it?
> [Hans] they're uninitialized so they have to be set otherwise bad things
> happen...
>
> Maybe we can go with this for now and think more about it when other classes
> are introduced?
> Thanks,
> Hans
>
>> Thanks
>> -Nagu
>>
>>> -----Original Message-----
>>> From: Hans Feldt [mailto:osafde...@gmail.com]
>>> Sent: 30 April 2014 10:31
>>> To: Nagendra Kumar; Praveen Malviya; hans.nordeb...@ericsson.com
>>> Cc: opensaf-devel@lists.sourceforge.net
>>> Subject: [PATCH 4 of 7] amfd: add SU constr/destr [#713]
>>>
>>> osaf/services/saf/amf/amfd/include/su.h | 17 +------
>>> osaf/services/saf/amf/amfd/su.cc | 74
>>> +++++++++++++++-----------------
>>> 2 files changed, 37 insertions(+), 54 deletions(-)
>>>
>>>
>>> diff --git a/osaf/services/saf/amf/amfd/include/su.h
>>> b/osaf/services/saf/amf/amfd/include/su.h
>>> --- a/osaf/services/saf/amf/amfd/include/su.h
>>> +++ b/osaf/services/saf/amf/amfd/include/su.h
>>> @@ -92,6 +92,9 @@ class AVD_SU {
>>> struct avd_sutype *su_type;
>>> AVD_SU *su_list_su_type_next;
>>>
>>> + AVD_SU() {};
>>> + AVD_SU(const SaNameT *dn);
>>> + ~AVD_SU();
>>> void set_su_failover(bool value);
>>> void dec_curr_stdby_si(void);
>>> void inc_curr_stdby_si(void);
>>> @@ -146,20 +149,6 @@ m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(cb, su,
>>> else su_node_ptr = i_su->su_on_node;
>>>
>>> /**
>>> - * Allocate SU memory and initialize attributes to defaults
>>> - * @param dn
>>> - *
>>> - * @return AVD_SU*
>>> - */
>>> -extern AVD_SU *avd_su_new(const SaNameT *dn);
>>> -
>>> -/**
>>> - * Free SU memory
>>> - * @param su
>>> - */
>>> -extern void avd_su_delete(AVD_SU *su);
>>> -
>>> -/**
>>> * Get SUs from IMM and create internal objects
>>> *
>>> * @return SaAisErrorT
>>> diff --git a/osaf/services/saf/amf/amfd/su.cc
>>> b/osaf/services/saf/amf/amfd/su.cc
>>> --- a/osaf/services/saf/amf/amfd/su.cc
>>> +++ b/osaf/services/saf/amf/amfd/su.cc
>>> @@ -31,51 +31,46 @@
>>>
>>> AmfDb<AVD_SU> *su_db = NULL;
>>>
>>> -AVD_SU *avd_su_new(const SaNameT *dn)
>>> -{
>>> - SaNameT sg_name;
>>> - AVD_SU *su;
>>> -
>>> - su = new AVD_SU();
>>> -
>>> - memcpy(su->name.value, dn->value, dn->length);
>>> - su->name.length = dn->length;
>>> - avsv_sanamet_init(dn, &sg_name, "safSg");
>>> - su->saAmfSUFailover = false;
>>> - su->term_state = false;
>>> - su->su_switch = AVSV_SI_TOGGLE_STABLE;
>>> - su->saAmfSUPreInstantiable = static_cast<SaBoolT>(false);
>>> - /* saAmfSUOperState is set when the SU is added to model depending
>>> on
>>> - * node state. Initialized to invalid due to filtering in
>>> avd_su_oper_state_set. */
>>> - su->saAmfSUOperState = static_cast<SaAmfOperationalStateT>(0);
>>> - su->saAmfSUPresenceState = SA_AMF_PRESENCE_UNINSTANTIATED;
>>> - su->saAmfSuReadinessState = SA_AMF_READINESS_OUT_OF_SERVICE;
>>> - su->su_is_external = false;
>>> -
>>> - return su;
>>> +AVD_SU::AVD_SU(const SaNameT *dn) {
>>> + memcpy(name.value, dn->value, sizeof(name.value));
>>> + name.length = dn->length;
>>> + saAmfSUFailover = false;
>>> + term_state = false;
>>> + su_switch = AVSV_SI_TOGGLE_STABLE;
>>> + saAmfSUPreInstantiable = static_cast<SaBoolT>(false);
>>> + saAmfSUOperState = SA_AMF_OPERATIONAL_DISABLED;
>>> + saAmfSUPresenceState = SA_AMF_PRESENCE_UNINSTANTIATED;
>>> + saAmfSuReadinessState = SA_AMF_READINESS_OUT_OF_SERVICE;
>>> + su_is_external = false;
>>> + sg_of_su = NULL;
>>> + su_on_node = NULL;
>>> + list_of_susi = NULL;
>>> + list_of_comp = NULL;
>>> + sg_list_su_next = NULL;
>>> + avnd_list_su_next = NULL;
>>> + su_type = NULL;
>>> + su_list_su_type_next = NULL;
>>> + saAmfSUHostedByNode.length = 0;
>>> }
>>>
>>> /**
>>> * Delete the SU from the model. Check point with peer. Send delete order
>>> * to node director. Delete all contained components.
>>> - *
>>> - * @param i_su
>>> */
>>> -void avd_su_delete(AVD_SU *su)
>>> -{
>>> - TRACE_ENTER2("'%s'", su->name.value);
>>> +AVD_SU::~AVD_SU() {
>>> + TRACE_ENTER2("'%s'", name.value);
>>>
>>> /* All the components under this SU should have been deleted
>>> * by now, just do the sanity check to confirm it is done
>>> */
>>> - osafassert(su->list_of_comp == NULL);
>>> + osafassert(list_of_comp == NULL);
>>> + osafassert(list_of_susi == NULL);
>>>
>>> - m_AVSV_SEND_CKPT_UPDT_ASYNC_RMV(avd_cb, su,
>>> AVSV_CKPT_AVD_SU_CONFIG);
>>> - avd_node_remove_su(su);
>>> - avd_sutype_remove_su(su);
>>> - su_db->erase(su);
>>> - avd_sg_remove_su(su);
>>> - delete su;
>>> + m_AVSV_SEND_CKPT_UPDT_ASYNC_RMV(avd_cb, this,
>>> AVSV_CKPT_AVD_SU_CONFIG);
>>> + avd_node_remove_su(this);
>>> + avd_sutype_remove_su(this);
>>> + su_db->erase(this);
>>> + avd_sg_remove_su(this);
>>>
>>> TRACE_LEAVE();
>>> }
>>> @@ -93,8 +88,7 @@ AVD_SU *avd_su_get_or_create(const SaNam
>>>
>>> if (su == NULL) {
>>> TRACE("'%s' does not exist, creating it", dn->value);
>>> - su = avd_su_new(dn);
>>> - osafassert(su != NULL);
>>> + su = new AVD_SU(dn);
>>> unsigned int rc = su_db->insert(su);
>>> osafassert(rc == NCSCC_RC_SUCCESS);
>>> }
>>> @@ -420,8 +414,7 @@ static AVD_SU *su_create(const SaNameT *
>>> ** but needs to get configuration attributes initialized.
>>> */
>>> if ((su = su_db->find(dn)) == NULL) {
>>> - if ((su = avd_su_new(dn)) == NULL)
>>> - goto done;
>>> + su = new AVD_SU(dn);
>>> } else
>>> TRACE("already created, refreshing config...");
>>>
>>> @@ -1489,7 +1482,7 @@ static void su_ccb_apply_delete_hdlr(str
>>> TRACE_ENTER2("'%s'", su->name.value);
>>>
>>> if (avd_cb->avail_state_avd != SA_AMF_HA_ACTIVE) {
>>> - avd_su_delete(su);
>>> + delete su;
>>> goto done;
>>> }
>>>
>>> @@ -1505,7 +1498,8 @@ static void su_ccb_apply_delete_hdlr(str
>>> avd_snd_op_req_msg(avd_cb, su_node_ptr, ¶m);
>>> }
>>>
>>> - avd_su_delete(su);
>>> + delete su;
>>> +
>>> if (AVD_SG_FSM_STABLE == sg->sg_fsm_state) {
>>> /*if su of uneqal rank has been delete and all SUs are of same
>>> rank then do screening
>>> for SI Distribution. */
>> ------------------------------------------------------------------------------
>> "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
>> Instantly run your Selenium tests across 300+ browser/OS combos. Get
>> unparalleled scalability from the best Selenium testing platform available.
>> Simple to use. Nothing to install. Get started now for free."
>> http://p.sf.net/sfu/SauceLabs
>> _______________________________________________
>> Opensaf-devel mailing list
>> Opensaf-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos. Get
unparalleled scalability from the best Selenium testing platform available.
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
