Re: [devel] [PATCH 3 of 8] cpa: Add support for extended SaNameT [#1574] v1

2016-08-23 Thread Vo Minh Hoang
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 Vo 
Cc: 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

2016-08-21 Thread A V Mahesh
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

2016-08-18 Thread Hoang Vo
 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);