Re: [devel] [PATCH 3 of 8] cpa: Add support for extended SaNameT [#1574] v1
Dear Mahesh, I would like to send updated patch following your comment. When this is minor comment, I send it as attached file. Sincerely, Hoang -Original Message- From: A V Mahesh [mailto:mahesh.va...@oracle.com] Sent: Monday, August 22, 2016 12:45 PM To: Hoang VoCc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 3 of 8] cpa: Add support for extended SaNameT [#1574] v1 Hi Hoang, ACK for [PATCH 3 of 8] with following minor comment I think API should return SA_AIS_ERR_TOO_BIG = 26 instead of SA_AIS_ERR_INVALID_PARAM ( please sync-up with other service return values ). Note : tested default functionality , LONG DN functionality not tested in full fledged -AVM On 8/18/2016 12:48 PM, Hoang Vo wrote: > osaf/libs/agents/saf/cpa/Makefile.am | 1 + > osaf/libs/agents/saf/cpa/cpa_api.c | 36 > osaf/libs/agents/saf/cpa/cpa_db.c| 2 + > osaf/libs/agents/saf/cpa/cpa_mds.c | 2 +- > osaf/libs/agents/saf/cpa/cpa_proc.c | 2 +- > osaf/libs/common/cpsv/include/cpa.h | 1 + > osaf/libs/common/cpsv/include/cpa_cb.h | 2 +- > osaf/libs/common/cpsv/include/cpa_proc.h | 2 +- > 8 files changed, 26 insertions(+), 22 deletions(-) > > > diff --git a/osaf/libs/agents/saf/cpa/Makefile.am > b/osaf/libs/agents/saf/cpa/Makefile.am > --- a/osaf/libs/agents/saf/cpa/Makefile.am > +++ b/osaf/libs/agents/saf/cpa/Makefile.am > @@ -22,6 +22,7 @@ noinst_LTLIBRARIES = libcpa.la > > libcpa_la_CPPFLAGS = \ > -DNCS_CPA=1 \ > + -DSA_EXTENDED_NAME_SOURCE \ > $(AM_CPPFLAGS) \ > -I$(top_srcdir)/osaf/libs/common/cpsv/include > > diff --git a/osaf/libs/agents/saf/cpa/cpa_api.c > b/osaf/libs/agents/saf/cpa/cpa_api.c > --- a/osaf/libs/agents/saf/cpa/cpa_api.c > +++ b/osaf/libs/agents/saf/cpa/cpa_api.c > @@ -870,19 +870,20 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > bool locked = false; > uint32_t time_out=0; > CPA_GLOBAL_CKPT_NODE *gc_node = NULL; > + SaConstStringT ckpt_name = NULL; > > TRACE_ENTER2("SaCkptCheckpointHandleT passed is %llx",ckptHandle); > - if ((checkpointName == NULL) || (checkpointHandle == NULL) || (checkpointName->length == 0)) { > + if ((checkpointName == NULL) || (checkpointHandle == NULL) || > +(osaf_extended_name_length(checkpointName) == 0)) { > TRACE_4("Cpa CkptOpen Api failed with return value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); > TRACE_LEAVE2("API return code = %u", rc); > return SA_AIS_ERR_INVALID_PARAM; > } > > - m_CPSV_SET_SANAMET(checkpointName); > + ckpt_name = osaf_extended_name_borrow(checkpointName); > > /* SA_AIS_ERR_INVALID_PARAM, bullet 4 in SAI-AIS-CKPT-B.02.02 > Section 3.6.1 saCkptCheckpointOpen() and saCkptCheckpointOpenAsync(), Return Values */ > -if (strncmp((const char *)checkpointName->value, "safCkpt=", 8) != 0) { > +if (strncmp(ckpt_name, "safCkpt=", 8) != 0) { > TRACE_4("Cpa CkptOpen:DN failed with return value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); > TRACE_LEAVE2("API return code = %u", rc); > return SA_AIS_ERR_INVALID_PARAM; [AVM] I think this should return SA_AIS_ERR_TOO_BIG = 26 ( please sync-up with other service return values ) > @@ -909,7 +910,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > > > /* Draft Validations */ > - rc = cpa_open_attr_validate(checkpointCreationAttributes, checkpointOpenFlags, checkpointName); > + rc = cpa_open_attr_validate(checkpointCreationAttributes, > +checkpointOpenFlags); > if (rc != SA_AIS_OK) { > /* No need to log, already logged inside the cpa_open_attr_validate */ > goto done; > @@ -965,7 +966,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > lc_node->cl_hdl = ckptHandle; > lc_node->open_flags = checkpointOpenFlags; > > - lc_node->ckpt_name = *checkpointName; > + lc_node->ckpt_name = strdup(ckpt_name); > > /* Add CPA_LOCAL_CKPT_NODE to lcl_ckpt_hdl_tree */ > proc_rc = cpa_lcl_ckpt_node_add(>lcl_ckpt_tree, lc_node); @@ > -984,7 +985,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > evt.info.cpnd.info.openReq.client_hdl = ckptHandle; > evt.info.cpnd.info.openReq.lcl_ckpt_hdl = lc_node->lcl_ckpt_hdl; > > - evt.info.cpnd.info.openReq.ckpt_name = *checkpointName; > + osaf_extended_name_lend(ckpt_name, > +_name); > > if (checkpointCreationAttributes) { > evt.info.cpnd.info.openReq.ckpt_attrib = > *checkpointCreationAttributes; @@ -1128,6 +1129,7 @@ gl_node_add_fail: > >lc_node_add_fail: > if (lc_node != NULL) { > + free((void *)lc_node->ckpt_name); > m_MMGR_FREE_CPA_LOCAL_CKPT_NODE(lc_node); > } > > @@ -1179,6 +1181,7 @@ SaAisErrorT
Re: [devel] [PATCH 3 of 8] cpa: Add support for extended SaNameT [#1574] v1
Hi Hoang, ACK for [PATCH 3 of 8] with following minor comment I think API should return SA_AIS_ERR_TOO_BIG = 26 instead of SA_AIS_ERR_INVALID_PARAM ( please sync-up with other service return values ). Note : tested default functionality , LONG DN functionality not tested in full fledged -AVM On 8/18/2016 12:48 PM, Hoang Vo wrote: > osaf/libs/agents/saf/cpa/Makefile.am | 1 + > osaf/libs/agents/saf/cpa/cpa_api.c | 36 > > osaf/libs/agents/saf/cpa/cpa_db.c| 2 + > osaf/libs/agents/saf/cpa/cpa_mds.c | 2 +- > osaf/libs/agents/saf/cpa/cpa_proc.c | 2 +- > osaf/libs/common/cpsv/include/cpa.h | 1 + > osaf/libs/common/cpsv/include/cpa_cb.h | 2 +- > osaf/libs/common/cpsv/include/cpa_proc.h | 2 +- > 8 files changed, 26 insertions(+), 22 deletions(-) > > > diff --git a/osaf/libs/agents/saf/cpa/Makefile.am > b/osaf/libs/agents/saf/cpa/Makefile.am > --- a/osaf/libs/agents/saf/cpa/Makefile.am > +++ b/osaf/libs/agents/saf/cpa/Makefile.am > @@ -22,6 +22,7 @@ noinst_LTLIBRARIES = libcpa.la > > libcpa_la_CPPFLAGS = \ > -DNCS_CPA=1 \ > + -DSA_EXTENDED_NAME_SOURCE \ > $(AM_CPPFLAGS) \ > -I$(top_srcdir)/osaf/libs/common/cpsv/include > > diff --git a/osaf/libs/agents/saf/cpa/cpa_api.c > b/osaf/libs/agents/saf/cpa/cpa_api.c > --- a/osaf/libs/agents/saf/cpa/cpa_api.c > +++ b/osaf/libs/agents/saf/cpa/cpa_api.c > @@ -870,19 +870,20 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > bool locked = false; > uint32_t time_out=0; > CPA_GLOBAL_CKPT_NODE *gc_node = NULL; > + SaConstStringT ckpt_name = NULL; > > TRACE_ENTER2("SaCkptCheckpointHandleT passed is %llx",ckptHandle); > - if ((checkpointName == NULL) || (checkpointHandle == NULL) || > (checkpointName->length == 0)) { > + if ((checkpointName == NULL) || (checkpointHandle == NULL) || > (osaf_extended_name_length(checkpointName) == 0)) { > TRACE_4("Cpa CkptOpen Api failed with return > value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); > TRACE_LEAVE2("API return code = %u", rc); > return SA_AIS_ERR_INVALID_PARAM; > } > > - m_CPSV_SET_SANAMET(checkpointName); > + ckpt_name = osaf_extended_name_borrow(checkpointName); > > /* SA_AIS_ERR_INVALID_PARAM, bullet 4 in SAI-AIS-CKPT-B.02.02 > Section 3.6.1 saCkptCheckpointOpen() and > saCkptCheckpointOpenAsync(), Return Values */ > -if (strncmp((const char *)checkpointName->value, "safCkpt=", 8) != > 0) { > +if (strncmp(ckpt_name, "safCkpt=", 8) != 0) { > TRACE_4("Cpa CkptOpen:DN failed with return > value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); > TRACE_LEAVE2("API return code = %u", rc); > return SA_AIS_ERR_INVALID_PARAM; [AVM] I think this should return SA_AIS_ERR_TOO_BIG = 26 ( please sync-up with other service return values ) > @@ -909,7 +910,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > > > /* Draft Validations */ > - rc = cpa_open_attr_validate(checkpointCreationAttributes, > checkpointOpenFlags, checkpointName); > + rc = cpa_open_attr_validate(checkpointCreationAttributes, > checkpointOpenFlags); > if (rc != SA_AIS_OK) { > /* No need to log, already logged inside the > cpa_open_attr_validate */ > goto done; > @@ -965,7 +966,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > lc_node->cl_hdl = ckptHandle; > lc_node->open_flags = checkpointOpenFlags; > > - lc_node->ckpt_name = *checkpointName; > + lc_node->ckpt_name = strdup(ckpt_name); > > /* Add CPA_LOCAL_CKPT_NODE to lcl_ckpt_hdl_tree */ > proc_rc = cpa_lcl_ckpt_node_add(>lcl_ckpt_tree, lc_node); > @@ -984,7 +985,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH > evt.info.cpnd.info.openReq.client_hdl = ckptHandle; > evt.info.cpnd.info.openReq.lcl_ckpt_hdl = lc_node->lcl_ckpt_hdl; > > - evt.info.cpnd.info.openReq.ckpt_name = *checkpointName; > + osaf_extended_name_lend(ckpt_name, > _name); > > if (checkpointCreationAttributes) { > evt.info.cpnd.info.openReq.ckpt_attrib = > *checkpointCreationAttributes; > @@ -1128,6 +1129,7 @@ gl_node_add_fail: > >lc_node_add_fail: > if (lc_node != NULL) { > + free((void *)lc_node->ckpt_name); > m_MMGR_FREE_CPA_LOCAL_CKPT_NODE(lc_node); > } > > @@ -1179,6 +1181,7 @@ SaAisErrorT saCkptCheckpointOpenAsync(Sa > CPA_LOCAL_CKPT_NODE *lc_node = NULL; > CPA_CLIENT_NODE *cl_node = NULL; > uint32_t proc_rc = NCSCC_RC_SUCCESS; > + SaConstStringT ckpt_name = NULL; > > TRACE_ENTER2("SaCkptCheckpointHandleT passed is %llx",ckptHandle); > > @@ -1188,19 +1191,17 @@ SaAisErrorT saCkptCheckpointOpenAsync(Sa > return SA_AIS_ERR_INVALID_PARAM; > } > >
[devel] [PATCH 3 of 8] cpa: Add support for extended SaNameT [#1574] v1
osaf/libs/agents/saf/cpa/Makefile.am | 1 + osaf/libs/agents/saf/cpa/cpa_api.c | 36 osaf/libs/agents/saf/cpa/cpa_db.c| 2 + osaf/libs/agents/saf/cpa/cpa_mds.c | 2 +- osaf/libs/agents/saf/cpa/cpa_proc.c | 2 +- osaf/libs/common/cpsv/include/cpa.h | 1 + osaf/libs/common/cpsv/include/cpa_cb.h | 2 +- osaf/libs/common/cpsv/include/cpa_proc.h | 2 +- 8 files changed, 26 insertions(+), 22 deletions(-) diff --git a/osaf/libs/agents/saf/cpa/Makefile.am b/osaf/libs/agents/saf/cpa/Makefile.am --- a/osaf/libs/agents/saf/cpa/Makefile.am +++ b/osaf/libs/agents/saf/cpa/Makefile.am @@ -22,6 +22,7 @@ noinst_LTLIBRARIES = libcpa.la libcpa_la_CPPFLAGS = \ -DNCS_CPA=1 \ + -DSA_EXTENDED_NAME_SOURCE \ $(AM_CPPFLAGS) \ -I$(top_srcdir)/osaf/libs/common/cpsv/include diff --git a/osaf/libs/agents/saf/cpa/cpa_api.c b/osaf/libs/agents/saf/cpa/cpa_api.c --- a/osaf/libs/agents/saf/cpa/cpa_api.c +++ b/osaf/libs/agents/saf/cpa/cpa_api.c @@ -870,19 +870,20 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH bool locked = false; uint32_t time_out=0; CPA_GLOBAL_CKPT_NODE *gc_node = NULL; + SaConstStringT ckpt_name = NULL; TRACE_ENTER2("SaCkptCheckpointHandleT passed is %llx",ckptHandle); - if ((checkpointName == NULL) || (checkpointHandle == NULL) || (checkpointName->length == 0)) { + if ((checkpointName == NULL) || (checkpointHandle == NULL) || (osaf_extended_name_length(checkpointName) == 0)) { TRACE_4("Cpa CkptOpen Api failed with return value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); TRACE_LEAVE2("API return code = %u", rc); return SA_AIS_ERR_INVALID_PARAM; } - m_CPSV_SET_SANAMET(checkpointName); + ckpt_name = osaf_extended_name_borrow(checkpointName); /* SA_AIS_ERR_INVALID_PARAM, bullet 4 in SAI-AIS-CKPT-B.02.02 Section 3.6.1 saCkptCheckpointOpen() and saCkptCheckpointOpenAsync(), Return Values */ -if (strncmp((const char *)checkpointName->value, "safCkpt=", 8) != 0) { +if (strncmp(ckpt_name, "safCkpt=", 8) != 0) { TRACE_4("Cpa CkptOpen:DN failed with return value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle); TRACE_LEAVE2("API return code = %u", rc); return SA_AIS_ERR_INVALID_PARAM; @@ -909,7 +910,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH /* Draft Validations */ - rc = cpa_open_attr_validate(checkpointCreationAttributes, checkpointOpenFlags, checkpointName); + rc = cpa_open_attr_validate(checkpointCreationAttributes, checkpointOpenFlags); if (rc != SA_AIS_OK) { /* No need to log, already logged inside the cpa_open_attr_validate */ goto done; @@ -965,7 +966,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH lc_node->cl_hdl = ckptHandle; lc_node->open_flags = checkpointOpenFlags; - lc_node->ckpt_name = *checkpointName; + lc_node->ckpt_name = strdup(ckpt_name); /* Add CPA_LOCAL_CKPT_NODE to lcl_ckpt_hdl_tree */ proc_rc = cpa_lcl_ckpt_node_add(>lcl_ckpt_tree, lc_node); @@ -984,7 +985,7 @@ SaAisErrorT saCkptCheckpointOpen(SaCkptH evt.info.cpnd.info.openReq.client_hdl = ckptHandle; evt.info.cpnd.info.openReq.lcl_ckpt_hdl = lc_node->lcl_ckpt_hdl; - evt.info.cpnd.info.openReq.ckpt_name = *checkpointName; + osaf_extended_name_lend(ckpt_name, _name); if (checkpointCreationAttributes) { evt.info.cpnd.info.openReq.ckpt_attrib = *checkpointCreationAttributes; @@ -1128,6 +1129,7 @@ gl_node_add_fail: lc_node_add_fail: if (lc_node != NULL) { + free((void *)lc_node->ckpt_name); m_MMGR_FREE_CPA_LOCAL_CKPT_NODE(lc_node); } @@ -1179,6 +1181,7 @@ SaAisErrorT saCkptCheckpointOpenAsync(Sa CPA_LOCAL_CKPT_NODE *lc_node = NULL; CPA_CLIENT_NODE *cl_node = NULL; uint32_t proc_rc = NCSCC_RC_SUCCESS; + SaConstStringT ckpt_name = NULL; TRACE_ENTER2("SaCkptCheckpointHandleT passed is %llx",ckptHandle); @@ -1188,19 +1191,17 @@ SaAisErrorT saCkptCheckpointOpenAsync(Sa return SA_AIS_ERR_INVALID_PARAM; } - /* Draft Validations */ - - m_CPSV_SET_SANAMET(checkpointName); + ckpt_name = osaf_extended_name_borrow(checkpointName); /* SA_AIS_ERR_INVALID_PARAM, bullet 4 in SAI-AIS-CKPT-B.02.02 Section 3.6.1 saCkptCheckpointOpen() and saCkptCheckpointOpenAsync(), Return Values */ -if (strncmp((const char *)checkpointName->value, "safCkpt=", 8) != 0) { +if (strncmp(ckpt_name, "safCkpt=", 8) != 0) { TRACE_4("cpa CkptOpen:DN Api failed with return value:%d,ckptHandle:%llx", SA_AIS_ERR_INVALID_PARAM, ckptHandle);