Hi Nagu,
I wrote a small test program that you can try to explore how
std::erase/std::remove operates. /Thanks HansN
On 09/09/2015 03:15 PM, Hans Nordebäck wrote:
Hi Nagu,
please see comment below./Thanks HansN
On 09/09/2015 02:10 PM, Nagendra Kumar wrote:
Please find my comment below:
-----Original Message-----
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: 09 September 2015 17:26
To: Nagendra Kumar; Praveen Malviya; gary....@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 10 of 10] amfd: Make AVD_SUTYPE a class [#1142]
see comment below/Thanks HansN
On 09/09/2015 01:53 PM, Nagendra Kumar wrote:
Thanks for the clarification, few more needed, please check below.
-----Original Message-----
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: 09 September 2015 17:09
To: Nagendra Kumar; Praveen Malviya; gary....@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 10 of 10] amfd: Make AVD_SUTYPE a class [#1142]
Hi Nagu,
please see my comment inlined with [HansN] /Thanks HansN
On 09/09/2015 01:13 PM, Nagendra Kumar wrote:
Please check my comment below
Thanks
-Nagu
-----Original Message-----
From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
Sent: 09 September 2015 15:59
To: Nagendra Kumar; Praveen Malviya; gary....@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 10 of 10] amfd: Make AVD_SUTYPE a class [#1142]
Hi Nagu,
please see comment inlined with [HansN] /Thanks HansN
On 09/09/2015 11:58 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 10 of 10] amfd: Make AVD_SUTYPE a class [#1142]
osaf/services/saf/amf/amfd/include/su.h | 3 +-
osaf/services/saf/amf/amfd/include/sutcomptype.h | 2 +-
osaf/services/saf/amf/amfd/include/sutype.h | 24 +++-
osaf/services/saf/amf/amfd/sgtype.cc | 2 +-
osaf/services/saf/amf/amfd/su.cc | 6 +-
osaf/services/saf/amf/amfd/sutcomptype.cc | 2 +-
osaf/services/saf/amf/amfd/sutype.cc | 108 +++++++++-------
----
--
7 files changed, 70 insertions(+), 77 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
@@ -35,6 +35,7 @@
#include "include/db_template.h"
class AVD_SG;
+class AVD_SUTYPE;
/**
* AMF director Service Unit representation.
@@ -94,7 +95,7 @@ class AVD_SU {
AVD_SU *sg_list_su_next; /* the next SU in the SG */
AVD_SU *avnd_list_su_next; /* the next SU in the
AvND */
- struct avd_sutype *su_type;
+ AVD_SUTYPE *su_type;
AVD_SU *su_list_su_type_next;
void set_su_failover(bool value); diff --git
a/osaf/services/saf/amf/amfd/include/sutcomptype.h
b/osaf/services/saf/amf/amfd/include/sutcomptype.h
--- a/osaf/services/saf/amf/amfd/include/sutcomptype.h
+++ b/osaf/services/saf/amf/amfd/include/sutcomptype.h
@@ -36,7 +36,7 @@ typedef struct {
} AVD_SUTCOMP_TYPE;
extern AmfDb<std::string, AVD_SUTCOMP_TYPE> *sutcomptype_db;
-SaAisErrorT avd_sutcomptype_config_get(SaNameT *sutype_name,
struct
avd_sutype *sut);
+SaAisErrorT avd_sutcomptype_config_get(SaNameT *sutype_name,
AVD_SUTYPE *sut);
void avd_sutcomptype_constructor(void);
#endif
diff --git a/osaf/services/saf/amf/amfd/include/sutype.h
b/osaf/services/saf/amf/amfd/include/sutype.h
--- a/osaf/services/saf/amf/amfd/include/sutype.h
+++ b/osaf/services/saf/amf/amfd/include/sutype.h
@@ -20,16 +20,24 @@
#include <saAis.h>
#include <su.h>
+#include <vector>
-struct avd_sutype {
- SaNameT name;
- SaUint32T saAmfSutIsExternal;
- SaUint32T saAmfSutDefSUFailover;
- SaNameT *saAmfSutProvidesSvcTypes; /* array of DNs, size in
number_svc_types */
- unsigned int number_svc_types; /* size of array
saAmfSutProvidesSvcTypes */
- AVD_SU *list_of_su;
+class AVD_SUTYPE {
+ public:
+ explicit AVD_SUTYPE(const SaNameT *dn);
+ SaNameT name {};
+ SaUint32T saAmfSutIsExternal {};
+ SaUint32T saAmfSutDefSUFailover {};
+ SaNameT *saAmfSutProvidesSvcTypes {}; /* array of DNs, size in
number_svc_types */
+ unsigned int number_svc_types {}; /* size of array
saAmfSutProvidesSvcTypes */
+ std::vector<AVD_SU*> list_of_su {};
+ private:
+ AVD_SUTYPE();
+ // disallow copy and assign
+ AVD_SUTYPE(const AVD_SUTYPE&); void operator=(const
+ AVD_SUTYPE&);
};
-extern AmfDb<std::string, avd_sutype> *sutype_db;
+extern AmfDb<std::string, AVD_SUTYPE> *sutype_db;
/**
* Get SaAmfSUType from IMM and create internal objects 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
@@ -84,7 +84,7 @@ static int is_config_valid(const SaNameT
char *parent;
SaUint32T value;
SaBoolT abool;
- struct avd_sutype *sut;
+ AVD_SUTYPE *sut;
const SaImmAttrValuesT_2 *attr;
if ((parent = strchr((char*)dn->value, ',')) == NULL)
{ 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
@@ -230,7 +230,7 @@ static int is_config_valid(const SaNameT
SaAmfAdminStateT admstate;
char *parent;
SaUint32T saAmfSutIsExternal;
- struct avd_sutype *sut = NULL;
+ AVD_SUTYPE *sut = NULL;
CcbUtilOperationData_t *tmp;
AVD_SG *sg;
@@ -416,7 +416,7 @@ static AVD_SU *su_create(const SaNameT *
{
int rc = -1;
AVD_SU *su;
- struct avd_sutype *sut;
+ AVD_SUTYPE *sut;
SaAisErrorT error;
TRACE_ENTER2("'%s'", dn->value); @@ -1691,7 +1691,7 @@
static void su_ccb_apply_modify_hdlr(str
su-
saAmfSUMaintenanceCampaign.value, su->name.value);
}
} else if (!strcmp(attr_mod->modAttr.attrName,
"saAmfSUType")) {
- struct avd_sutype *sut;
+ AVD_SUTYPE *sut;
SaNameT sutype_name = *(SaNameT*)
attr_mod-
modAttr.attrValues[0];
TRACE("Modified saAmfSUType from '%s' to
'%s'", su-
saAmfSUType.value, sutype_name.value);
sut = sutype_db-
find(Amf::to_string(&sutype_name));
diff --git a/osaf/services/saf/amf/amfd/sutcomptype.cc
b/osaf/services/saf/amf/amfd/sutcomptype.cc
--- a/osaf/services/saf/amf/amfd/sutcomptype.cc
+++ b/osaf/services/saf/amf/amfd/sutcomptype.cc
@@ -65,7 +65,7 @@ static int is_config_valid(const SaNameT
return 1;
}
-SaAisErrorT avd_sutcomptype_config_get(SaNameT *sutype_name,
struct
avd_sutype *sut)
+SaAisErrorT avd_sutcomptype_config_get(SaNameT *sutype_name,
AVD_SUTYPE *sut)
{
AVD_SUTCOMP_TYPE *sutcomptype;
SaAisErrorT error;
diff --git a/osaf/services/saf/amf/amfd/sutype.cc
b/osaf/services/saf/amf/amfd/sutype.cc
--- a/osaf/services/saf/amf/amfd/sutype.cc
+++ b/osaf/services/saf/amf/amfd/sutype.cc
@@ -27,37 +27,41 @@
#include <cluster.h>
#include <ntf.h>
#include <proc.h>
+#include <algorithm>
-AmfDb<std::string, avd_sutype> *sutype_db = NULL;
+AmfDb<std::string, AVD_SUTYPE> *sutype_db = NULL;
-static struct avd_sutype *sutype_new(const SaNameT *dn)
+//
+AVD_SUTYPE::AVD_SUTYPE(const SaNameT *dn) {
+ memcpy(&name.value, dn->value, dn->length);
+ name.length = dn->length;
+}
+
+static AVD_SUTYPE *sutype_new(const SaNameT *dn)
{
- struct avd_sutype *sutype = new avd_sutype();
-
- memcpy(sutype->name.value, dn->value, dn->length);
- sutype->name.length = dn->length;
+ AVD_SUTYPE *sutype = new AVD_SUTYPE(dn);
return sutype;
}
-static void sutype_delete(struct avd_sutype **sutype)
+static void sutype_delete(AVD_SUTYPE **sutype)
{
- osafassert(NULL == (*sutype)->list_of_su);
+ osafassert(true == (*sutype)->list_of_su.empty());
delete [] (*sutype)->saAmfSutProvidesSvcTypes;
delete *sutype;
*sutype = NULL;
}
-static void sutype_db_add(struct avd_sutype *sutype)
+static void sutype_db_add(AVD_SUTYPE *sutype)
{
unsigned int rc =
sutype_db->insert(Amf::to_string(&sutype-
name),sutype);
osafassert(rc == NCSCC_RC_SUCCESS);
}
-static struct avd_sutype *sutype_create(const SaNameT *dn, const
SaImmAttrValuesT_2 **attributes)
+static AVD_SUTYPE *sutype_create(const SaNameT *dn, const
SaImmAttrValuesT_2 **attributes)
{
const SaImmAttrValuesT_2 *attr;
- struct avd_sutype *sutype;
+ AVD_SUTYPE *sutype;
int rc = 0;
unsigned i = 0;
SaAisErrorT error;
@@ -166,7 +170,7 @@ static int is_config_valid(const SaNameT
SaAisErrorT avd_sutype_config_get(void)
{
- struct avd_sutype *sut;
+ AVD_SUTYPE *sut;
SaAisErrorT error;
SaImmSearchHandleT searchHandle;
SaImmSearchParametersT_2 searchParam; @@ -228,7 +232,7
@@
static
void sutype_ccb_apply_modify_hdlr
int i = 0;
TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, opdata-
objectName.value);
- avd_sutype *sut = sutype_db->find(Amf::to_string(&opdata-
objectName));
+ AVD_SUTYPE *sut = sutype_db->find(Amf::to_string(&opdata-
objectName));
while ((attr_mod =
opdata->param.modify.attrMods[i++]) !=
NULL) {
if (!strcmp(attr_mod->modAttr.attrName,
"saAmfSutDefSUFailover")) {
@@ -239,7 +243,7 @@ static void sutype_ccb_apply_modify_hdlr
/* Modify saAmfSUFailover for SUs which had
inherited
saAmfSutDefSUFailover.
Modification will not be done for the
NPI SU
*/
if (old_value != sut->saAmfSutDefSUFailover) {
- for (AVD_SU *su = sut->list_of_su; su;
su = su-
su_list_su_type_next) {
+ for (const auto& su : sut->list_of_su) {
if ((!su-
saAmfSUFailover_configured)
&& (su->saAmfSUPreInstantiable)) {
su-
set_su_failover(static_cast<bool>(sut->saAmfSutDefSUFailover));
}
@@ -255,7 +259,7 @@ static void sutype_ccb_apply_modify_hdlr
static void sutype_ccb_apply_cb(CcbUtilOperationData_t
*opdata)
{
- struct avd_sutype *sut;
+ AVD_SUTYPE *sut;
TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, opdata-
objectName.value);
@@ -291,7 +295,7 @@ static SaAisErrorT sutype_ccb_completed_
SaAisErrorT rc = SA_AIS_OK;
const SaImmAttrModificationT_2 *attr_mod;
int i = 0;
- avd_sutype *sut = sutype_db->find(Amf::to_string(&opdata-
objectName));
+ AVD_SUTYPE *sut = sutype_db->find(Amf::to_string(&opdata-
objectName));
TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, opdata-
objectName.value);
while ((attr_mod =
opdata->param.modify.attrMods[i++]) !=
NULL) { @@ -311,7 +315,7 @@ static SaAisErrorT
sutype_ccb_completed_
goto done;
}
- for (AVD_SU *su = sut->list_of_su; su; su = su-
su_list_su_type_next) {
+ for (const auto& su : sut->list_of_su) {
if (su->saAmfSUFailover_configured)
continue;
@@ -341,8 +345,7 @@ done:
static SaAisErrorT
sutype_ccb_completed_cb(CcbUtilOperationData_t
*opdata)
{
SaAisErrorT rc = SA_AIS_ERR_BAD_OPERATION;
- struct avd_sutype *sut;
- AVD_SU *su;
+ AVD_SUTYPE *sut;
bool su_exist = false;
CcbUtilOperationData_t *t_opData;
@@ -358,24 +361,23 @@ static SaAisErrorT sutype_ccb_completed_
break;
case CCBUTIL_DELETE:
sut = sutype_db->find(Amf::to_string(&opdata-
objectName));
- if (NULL != sut->list_of_su) {
- /* check whether there exists a delete
operation for
- * each of the SU in the su_type list in the
current CCB
- */
- su = sut->list_of_su;
- while (su != NULL) {
- t_opData =
ccbutil_getCcbOpDataByDN(opdata->ccbId, &su->name);
- if ((t_opData == NULL) || (t_opData-
operationType != CCBUTIL_DELETE)) {
- su_exist = true;
- break;
- }
- su = su->su_list_su_type_next;
- }
- if (su_exist == true) {
- report_ccb_validation_error(opdata,
"SaAmfSUType '%s'is in use",sut->name.value);
- goto done;
+
+ /* check whether there exists a delete operation for
+ * each of the SU in the su_type list in the current CCB
+ */
+
+ for (const auto& su : sut->list_of_su) {
+ t_opData =
ccbutil_getCcbOpDataByDN(opdata-
ccbId, &su->name);
+ if ((t_opData == NULL) || (t_opData-
operationType
!= CCBUTIL_DELETE)) {
+ su_exist = true;
+ break;
}
}
+
+ if (su_exist == true) {
+ report_ccb_validation_error(opdata,
"SaAmfSUType
'%s'is in use",sut->name.value);
+ goto done;
+ }
rc = SA_AIS_OK;
break;
default:
@@ -391,41 +393,23 @@ done:
/* Add SU to list in SU Type */
void avd_sutype_add_su(AVD_SU* su)
{
- struct avd_sutype *sut = su->su_type;
- su->su_list_su_type_next = sut->list_of_su;
- sut->list_of_su = su;
+ AVD_SUTYPE *sut = su->su_type;
+ sut->list_of_su.push_back(su);
}
-void avd_sutype_remove_su(AVD_SU* su) -{
- AVD_SU *i_su = NULL;
- AVD_SU *prev_su = NULL;
+void avd_sutype_remove_su(AVD_SU* su) {
+ AVD_SUTYPE *su_type = su->su_type;
- if (su->su_type != NULL) {
- i_su = su->su_type->list_of_su;
-
- while ((i_su != NULL) && (i_su != su)) {
- prev_su = i_su;
- i_su = i_su->su_list_su_type_next;
- }
-
- if (i_su == su) {
- if (prev_su == NULL) {
- su->su_type->list_of_su = su-
su_list_su_type_next;
- } else {
- prev_su->su_list_su_type_next = su-
su_list_su_type_next;
- }
-
- su->su_list_su_type_next = NULL;
- su->su_type = NULL;
- }
- }
+ if (su_type != nullptr) {
+
su_type->list_of_su.erase(std::remove(su_type->list_of_su.begin(),
+
+ su_type->list_of_su.end(), su), su_type-
list_of_su.end());
[Nagu]: Looks complex to understand, can it be done as like #9th
of patch
series?
[HansN] It can be done like #9th patch, the reason std::find is
used in
#9 is because
of the osafassert, that checks if si exists in
svc_type->list_of_si, this is not needed in #10.
The use of std::remove, in #10, is to show an alternative and (in
my
opinion) easier way
to express the remove/erase idiom.
[Nagu]: I got confused with erase taking first element as 'an
element after
remove' and then it erases from
'removed item till end of the list i.e. range ?' Am I right ?
can
we refactor it
?
[HansN]
The std::remove doesn't remove anything it returns a new iterator to
a new end where objects to be erased has been transformed. Please see
e.g.
http://www.cplusplus.com/reference/algorithm/remove/?kw=remove
[Nagu]: Yes, I had read it before and I found it confusing. That
means the
statement is equivalent to :
su_type->list_of_su.erase(Iterator, su_type-> list_of_su.end()) So,
does it erase a range of elements from iterator till end ?
[HansN] yes, the elements to be erased are at the new iterator till
end.
[Nagu]: The function is supposed to remove one SU from SU list. It
looks that it means
it removes SUs starting from SU5(say) till SU10(if SU10 is the
last)[If we have SU1 to SU10
and we need to remove SU5].
[HansN] No, as an example say list_of_su looks as below before
std::remove, (list_of_su are pointers to su):
SU1 SU3 SU5 SU6 SU7
if su is say pointer to SU3 in the call below to std::remove:
std::remove(su_type->list_of_su.begin(), su_type->list_of_su.end(), su)
SU1 SU5 SU6 SU7 SU3
-------------------------^
The iterator returned by std::remove will be at SU3. So the erase will
remove only SU3.
+ }
}
void avd_sutype_constructor(void)
{
- sutype_db = new AmfDb<std::string, avd_sutype>;
+ sutype_db = new AmfDb<std::string, AVD_SUTYPE>;
avd_class_impl_set("SaAmfSUBaseType", NULL, NULL,
avd_imm_default_OK_completed_cb, NULL);
avd_class_impl_set("SaAmfSUType", NULL, NULL,
#include <vector>
#include <algorithm>
#include <iostream>
#include <string.h>
// g++ -std=gnu++11 -g -o test_vec1 test_vec1.cc
std::vector<char*> myVec;
void remove_sg(char *sg) {
if (sg != nullptr) {
std::cout << "Erasing " << sg << std::endl;
myVec.erase(std::remove(myVec.begin(), myVec.end(), sg), myVec.end());
}
}
int main()
{
char *ptr;
char *ptr3;
for (const char* i : myVec) {
std::cout << *i << " " << std::endl;
}
ptr = new char[20] {};
strcpy(ptr, "SU1");
myVec.push_back(ptr);
ptr = new char[20] {};
strcpy(ptr, "SU2");
myVec.push_back(ptr);
ptr3 = new char[20] {};
strcpy(ptr3, "SU3");
myVec.push_back(ptr3);
ptr = new char[20] {};
strcpy(ptr, "SU4");
myVec.push_back(ptr);
ptr = new char[20] {};
strcpy(ptr, "SU5");
myVec.push_back(ptr);
for (const char *i : myVec) {
std::cout << i << std::endl;
}
remove_sg(ptr3);
std::cout << "after remove of SU3" << std::endl;
for (const char *i : myVec) {
std::cout << i << std::endl;
}
return 0;
}
------------------------------------------------------------------------------
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
