Re: [devel] [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]

2020-02-04 Thread Thuan Tran
Hi Alex,

OK, you can keep strncpy with len + 1.
No more comment from me.

From: Alex Jones 
Sent: Tuesday, February 4, 2020 9:40 PM
To: Thuan Tran ; Vu Minh Nguyen 

Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]


Hi ThuanTr,

I will add fclose(). Good catch.

We can't leave the original code in SmfUtils.cc because it fails to compile 
in gcc 9.x. The compiler complains that you are only copying the length of the 
string, so the output is not null terminated (even though the next line null 
terminates it). We could change the code to use memcpy instead. That would make 
it clearer that we are not intending to null terminate with the function call, 
and are doing it ourselves in the next line.

Alex
On 2/3/20 9:28 PM, Tran Thuan wrote:

NOTICE: This email was received from an EXTERNAL sender


Hi Alex,

About test_ntf_imcn.cc, please update following too
Since you add “return” then static code check report leak “ f ”.

@@ -6202,6 +6202,7 @@ __attribute__((constructor)) static void 
ntf_imcn_constructor(void) {
 snprintf(cp_cmd, sizeof(cp_cmd), "cp ");
 if ((strlen(line) - 1) > (sizeof(cp_cmd) - sizeof("cp "))) {
   printf("line: %s too long", line);
+  fclose(f);
   return;
 }

About SmfUtils.cc:

- strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+ strncpy(*((SaStringT *)*i_value), i_str, len + 1);
(*((SaStringT *)*i_value))[len] = '\0';

=> strncpy with “len + 1” then later overwrite with ‘\0’.
I suggest strncpy with “len” as original code to avoid redundant changes.

Best Regards,
ThuanTr

From: Alex Jones 
Sent: Monday, February 3, 2020 10:39 PM
To: thuan.t...@dektech.com.au; 
vu.m.ngu...@dektech.com.au
Cc: 
opensaf-devel@lists.sourceforge.net;
 Alex Jones 
Subject: [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]

Rework fixes in NTF and SMF.
---
src/ntf/apitest/test_ntf_imcn.cc | 2 +-
src/smf/smfd/SmfUtils.cc | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ntf/apitest/test_ntf_imcn.cc b/src/ntf/apitest/test_ntf_imcn.cc
index 51b9076c6..04f155074 100644
--- a/src/ntf/apitest/test_ntf_imcn.cc
+++ b/src/ntf/apitest/test_ntf_imcn.cc
@@ -1140,7 +1140,7 @@ static SaAisErrorT set_add_info(
>additionalInfo[idx].infoValue);
if (error == SA_AIS_OK) {
strcpy(reinterpret_cast(temp), infoValue);
- temp[strlen(infoValue) - 1] = '\0';
+ //temp[strlen(infoValue)] = '\0';
nHeader->additionalInfo[idx].infoId = infoId;
nHeader->additionalInfo[idx].infoType = SA_NTF_VALUE_STRING;
}
diff --git a/src/smf/smfd/SmfUtils.cc b/src/smf/smfd/SmfUtils.cc
index 2d539e7c2..f1593b4cf 100644
--- a/src/smf/smfd/SmfUtils.cc
+++ b/src/smf/smfd/SmfUtils.cc
@@ -993,7 +993,7 @@ bool smf_stringToValue(SaImmValueTypeT i_type, 
SaImmAttrValueT *i_value,
len = strlen(i_str);
*i_value = malloc(sizeof(SaStringT));
*((SaStringT *)*i_value) = (SaStringT)malloc(len + 1);
- strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+ strncpy(*((SaStringT *)*i_value), i_str, len + 1);
(*((SaStringT *)*i_value))[len] = '\0';
break;
case SA_IMM_ATTR_SAANYT:
--
2.21.1



Notice: This e-mail together with any attachments may contain information of 
Ribbon Communications Inc. that is confidential and/or proprietary for the sole 
use of the intended recipient. Any review, disclosure, reliance or distribution 
by others or forwarding without express permission is strictly prohibited. If 
you are not the intended recipient, please notify the sender immediately and 
then delete all copies, including any attachments.


___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]

2020-02-04 Thread Alex Jones
   Hi ThuanTr,

   I will add fclose(). Good catch.

   We can't leave the original code in SmfUtils.cc because it fails to
   compile in gcc 9.x. The compiler complains that you are only copying
   the length of the string, so the output is not null terminated (even
   though the next line null terminates it). We could change the code to
   use memcpy instead. That would make it clearer that we are not
   intending to null terminate with the function call, and are doing it
   ourselves in the next line.

   Alex

   On 2/3/20 9:28 PM, Tran Thuan wrote:
 __

   NOTICE: This email was received from an EXTERNAL sender
 __

   Hi Alex,


   About test_ntf_imcn.cc, please update following too

   Since you add "return" then static code check report leak " f ".


   @@ -6202,6 +6202,7 @@ __attribute__((constructor)) static void
   ntf_imcn_constructor(void) {

snprintf(cp_cmd, sizeof(cp_cmd), "cp ");

if ((strlen(line) - 1) > (sizeof(cp_cmd) - sizeof("cp ")))
   {

  printf("line: %s too long", line);

   +  fclose(f);

  return;

}


   About SmfUtils.cc:


   - strncpy(*((SaStringT *)*i_value), i_str, len - 1);
   + strncpy(*((SaStringT *)*i_value), i_str, len + 1);
   (*((SaStringT *)*i_value))[len] = '\0';


   => strncpy with "len + 1" then later overwrite with `\0'.

   I suggest strncpy with "len" as original code to avoid redundant
   changes.


   Best Regards,

   ThuanTr


   From: Alex Jones [1]
   Sent: Monday, February 3, 2020 10:39 PM
   To: [2]thuan.t...@dektech.com.au; [3]vu.m.ngu...@dektech.com.au
   Cc: [4]opensaf-devel@lists.sourceforge.net; Alex Jones
   [5]
   Subject: [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]


   Rework fixes in NTF and SMF.
   ---
   src/ntf/apitest/test_ntf_imcn.cc | 2 +-
   src/smf/smfd/SmfUtils.cc | 2 +-
   2 files changed, 2 insertions(+), 2 deletions(-)
   diff --git a/src/ntf/apitest/test_ntf_imcn.cc
   b/src/ntf/apitest/test_ntf_imcn.cc
   index 51b9076c6..04f155074 100644
   --- a/src/ntf/apitest/test_ntf_imcn.cc
   +++ b/src/ntf/apitest/test_ntf_imcn.cc
   @@ -1140,7 +1140,7 @@ static SaAisErrorT set_add_info(
   >additionalInfo[idx].infoValue);
   if (error == SA_AIS_OK) {
   strcpy(reinterpret_cast(temp), infoValue);
   - temp[strlen(infoValue) - 1] = '\0';
   + //temp[strlen(infoValue)] = '\0';
   nHeader->additionalInfo[idx].infoId = infoId;
   nHeader->additionalInfo[idx].infoType = SA_NTF_VALUE_STRING;
   }
   diff --git a/src/smf/smfd/SmfUtils.cc b/src/smf/smfd/SmfUtils.cc
   index 2d539e7c2..f1593b4cf 100644
   --- a/src/smf/smfd/SmfUtils.cc
   +++ b/src/smf/smfd/SmfUtils.cc
   @@ -993,7 +993,7 @@ bool smf_stringToValue(SaImmValueTypeT i_type,
   SaImmAttrValueT *i_value,
   len = strlen(i_str);
   *i_value = malloc(sizeof(SaStringT));
   *((SaStringT *)*i_value) = (SaStringT)malloc(len + 1);
   - strncpy(*((SaStringT *)*i_value), i_str, len - 1);
   + strncpy(*((SaStringT *)*i_value), i_str, len + 1);
   (*((SaStringT *)*i_value))[len] = '\0';
   break;
   case SA_IMM_ATTR_SAANYT:
   --
   2.21.1
   ___

   Notice: This e-mail together with any attachments may contain
   information of Ribbon Communications Inc. that is confidential and/or
   proprietary for the sole use of the intended recipient. Any review,
   disclosure, reliance or distribution by others or forwarding without
   express permission is strictly prohibited. If you are not the intended
   recipient, please notify the sender immediately and then delete all
   copies, including any attachments.
   ___

References

   1. mailto:ajo...@rbbn.com
   2. mailto:thuan.t...@dektech.com.au
   3. mailto:vu.m.ngu...@dektech.com.au
   4. mailto:opensaf-devel@lists.sourceforge.net
   5. mailto:ajo...@rbbn.com


0x0023444D652FA1D5.asc
Description: application/pgp-keys


signature.asc
Description: OpenPGP digital signature
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]

2020-02-03 Thread Tran Thuan
Hi Alex,

 

About test_ntf_imcn.cc, please update following too

Since you add “return” then static code check report leak “ f ”.

 

@@ -6202,6 +6202,7 @@ __attribute__((constructor)) static void 
ntf_imcn_constructor(void) {

 snprintf(cp_cmd, sizeof(cp_cmd), "cp ");

 if ((strlen(line) - 1) > (sizeof(cp_cmd) - sizeof("cp "))) {

   printf("line: %s too long", line);

+  fclose(f);

   return;

 }

 

About SmfUtils.cc:

 

- strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+ strncpy(*((SaStringT *)*i_value), i_str, len + 1);
(*((SaStringT *)*i_value))[len] = '\0';

 

=> strncpy with “len + 1” then later overwrite with ‘\0’.

I suggest strncpy with “len” as original code to avoid redundant changes.

 

Best Regards,

ThuanTr

 

From: Alex Jones  
Sent: Monday, February 3, 2020 10:39 PM
To: thuan.t...@dektech.com.au; vu.m.ngu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Alex Jones 
Subject: [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]

 

Rework fixes in NTF and SMF.
---
src/ntf/apitest/test_ntf_imcn.cc | 2 +-
src/smf/smfd/SmfUtils.cc | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ntf/apitest/test_ntf_imcn.cc b/src/ntf/apitest/test_ntf_imcn.cc
index 51b9076c6..04f155074 100644
--- a/src/ntf/apitest/test_ntf_imcn.cc
+++ b/src/ntf/apitest/test_ntf_imcn.cc
@@ -1140,7 +1140,7 @@ static SaAisErrorT set_add_info(
>additionalInfo[idx].infoValue);
if (error == SA_AIS_OK) {
strcpy(reinterpret_cast(temp), infoValue);
- temp[strlen(infoValue) - 1] = '\0';
+ //temp[strlen(infoValue)] = '\0';
nHeader->additionalInfo[idx].infoId = infoId;
nHeader->additionalInfo[idx].infoType = SA_NTF_VALUE_STRING;
}
diff --git a/src/smf/smfd/SmfUtils.cc b/src/smf/smfd/SmfUtils.cc
index 2d539e7c2..f1593b4cf 100644
--- a/src/smf/smfd/SmfUtils.cc
+++ b/src/smf/smfd/SmfUtils.cc
@@ -993,7 +993,7 @@ bool smf_stringToValue(SaImmValueTypeT i_type, 
SaImmAttrValueT *i_value,
len = strlen(i_str);
*i_value = malloc(sizeof(SaStringT));
*((SaStringT *)*i_value) = (SaStringT)malloc(len + 1);
- strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+ strncpy(*((SaStringT *)*i_value), i_str, len + 1);
(*((SaStringT *)*i_value))[len] = '\0';
break;
case SA_IMM_ATTR_SAANYT:
-- 
2.21.1



  _  

Notice: This e-mail together with any attachments may contain information of 
Ribbon Communications Inc. that is confidential and/or proprietary for the sole 
use of the intended recipient. Any review, disclosure, reliance or distribution 
by others or forwarding without express permission is strictly prohibited. If 
you are not the intended recipient, please notify the sender immediately and 
then delete all copies, including any attachments.

  _  


___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


[devel] [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]

2020-02-03 Thread Alex Jones
Rework fixes in NTF and SMF.
---
 src/ntf/apitest/test_ntf_imcn.cc | 2 +-
 src/smf/smfd/SmfUtils.cc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ntf/apitest/test_ntf_imcn.cc b/src/ntf/apitest/test_ntf_imcn.cc
index 51b9076c6..04f155074 100644
--- a/src/ntf/apitest/test_ntf_imcn.cc
+++ b/src/ntf/apitest/test_ntf_imcn.cc
@@ -1140,7 +1140,7 @@ static SaAisErrorT set_add_info(
   >additionalInfo[idx].infoValue);
   if (error == SA_AIS_OK) {
 strcpy(reinterpret_cast(temp), infoValue);
-temp[strlen(infoValue) - 1] = '\0';
+//temp[strlen(infoValue)] = '\0';
 nHeader->additionalInfo[idx].infoId = infoId;
 nHeader->additionalInfo[idx].infoType = SA_NTF_VALUE_STRING;
   }
diff --git a/src/smf/smfd/SmfUtils.cc b/src/smf/smfd/SmfUtils.cc
index 2d539e7c2..f1593b4cf 100644
--- a/src/smf/smfd/SmfUtils.cc
+++ b/src/smf/smfd/SmfUtils.cc
@@ -993,7 +993,7 @@ bool smf_stringToValue(SaImmValueTypeT i_type, 
SaImmAttrValueT *i_value,
   len = strlen(i_str);
   *i_value = malloc(sizeof(SaStringT));
   *((SaStringT *)*i_value) = (SaStringT)malloc(len + 1);
-  strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+  strncpy(*((SaStringT *)*i_value), i_str, len + 1);
   (*((SaStringT *)*i_value))[len] = '\0';
   break;
 case SA_IMM_ATTR_SAANYT:
-- 
2.21.1


---
Notice: This e-mail together with any attachments may contain information of 
Ribbon Communications Inc. that
is confidential and/or proprietary for the sole use of the intended recipient.  
Any review, disclosure, reliance or
distribution by others or forwarding without express permission is strictly 
prohibited.  If you are not the intended
recipient, please notify the sender immediately and then delete all copies, 
including any attachments.
---

___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel