Re: [devel] [PATCH 1 of 1] log: fix cppcheck with performance severity [#1975]

2016-08-31 Thread Canh Truong
Hi Mahesh,

I run cppcheck with latest version 1.75. I got from:
https://sourceforge.net/projects/cppcheck/files/cppcheck/1.75/
And build, then run with option --enable=performance.

Regards,
Canh.

-Original Message-
From: A V Mahesh [mailto:mahesh.va...@oracle.com] 
Sent: Thursday, September 01, 2016 11:03 AM
To: Canh Van Truong; vu.m.ngu...@dektech.com.au; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1 of 1] log: fix cppcheck with performance severity
[#1975]

  Hi Canh Van Truong,

  Can you please share the suggestion reported by cppcheck.

-AVM


On 8/31/2016 2:19 PM, Canh Van Truong wrote:
>   osaf/services/saf/logsv/lgs/lgs_config.cc |  35
--
>   osaf/services/saf/logsv/lgs/lgs_file.cc   |   4 +-
>   2 files changed, 16 insertions(+), 23 deletions(-)
>
>
> Fix performance reported by cppcheck.
>
> diff --git a/osaf/services/saf/logsv/lgs/lgs_config.cc 
> b/osaf/services/saf/logsv/lgs/lgs_config.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_config.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_config.cc
> @@ -137,32 +137,25 @@ typedef struct _lgs_conf_t {
> lgs_conf_flg_t logDataGroupname_cnfflag;
> lgs_conf_flg_t logStreamFileFormat_cnfflag;
>   
> -  _lgs_conf_t() {
> -/*
> - * For the following flags, LGS_CNF_DEF means that no external
> - * configuration exists and the corresponding attributes hard-coded
> - * default value is used.Is set to false if configuration is found in
> - * IMM object or environment variable.
> - * See function lgs_logconf_get() for more info.
> - */
> +  _lgs_conf_t()
> +  : logRootDirectory(lgs_conf_def.logRootDirectory),
> +logRootDirectory_cnfflag(LGS_CNF_DEF),
> +logMaxLogrecsize_cnfflag(LGS_CNF_DEF),
> +logStreamSystemHighLimit_cnfflag(LGS_CNF_DEF),
> +logStreamSystemLowLimit_cnfflag(LGS_CNF_DEF),
> +logStreamAppHighLimit_cnfflag(LGS_CNF_DEF),
> +logStreamAppLowLimit_cnfflag(LGS_CNF_DEF),
> +logMaxApplicationStreams_cnfflag(LGS_CNF_DEF),
> +logFileIoTimeout_cnfflag(LGS_CNF_DEF),
> +logFileSysConfig_cnfflag(LGS_CNF_DEF),
> +logDataGroupname_cnfflag(LGS_CNF_DEF),
> +logStreamFileFormat_cnfflag(LGS_CNF_DEF)
> +  {
>   OpenSafLogConfig_object_exist = false;
> -logRootDirectory_cnfflag = LGS_CNF_DEF;
> -logStreamSystemHighLimit_cnfflag = LGS_CNF_DEF;
> -logStreamSystemLowLimit_cnfflag = LGS_CNF_DEF;
> -logStreamAppHighLimit_cnfflag = LGS_CNF_DEF;
> -logStreamAppLowLimit_cnfflag = LGS_CNF_DEF;
> -logDataGroupname_cnfflag = LGS_CNF_DEF;
>   /*
>* The following attributes cannot be configured in the config file
>* Will be set to false if the attribute exists in the IMM config
object
>*/
> -logMaxLogrecsize_cnfflag = LGS_CNF_DEF;
> -logMaxApplicationStreams_cnfflag = LGS_CNF_DEF;
> -logFileIoTimeout_cnfflag = LGS_CNF_DEF;
> -logFileSysConfig_cnfflag = LGS_CNF_DEF;
> -logStreamFileFormat_cnfflag = LGS_CNF_DEF;
> -
> -logRootDirectory = lgs_conf_def.logRootDirectory;
>   (void) strcpy(logDataGroupname, lgs_conf_def.logDataGroupname);
>   (void) strcpy(logStreamFileFormat,
lgs_conf_def.logStreamFileFormat);
>   logMaxLogrecsize = lgs_conf_def.logMaxLogrecsize; diff --git 
> a/osaf/services/saf/logsv/lgs/lgs_file.cc 
> b/osaf/services/saf/logsv/lgs/lgs_file.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_file.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_file.cc
> @@ -51,11 +51,11 @@ struct file_communicate {
> size_t outdata_size;
> void *outdata_ptr;  /* Out data from handlers */
>   
> -  file_communicate() {
> +  file_communicate() : request_code(LGSF_NOREQ)  {
>   answer_f = false;
>   request_f = false;
>   timeout_f = false;
> -request_code = LGSF_NOREQ;
>   return_code = LGSF_NORETC;
>   indata_ptr = NULL;
>   outdata_ptr = NULL;



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


Re: [devel] [PATCH 1 of 1] log: fix errors reported by cppcheck version 1.75 [#1985]

2016-08-31 Thread Canh Truong
Hi  Mahesh,

Please see my comment inline.

Regards,
Canh.

-Original Message-
From: A V Mahesh [mailto:mahesh.va...@oracle.com] 
Sent: Thursday, September 01, 2016 10:59 AM
To: Canh Van Truong; vu.m.ngu...@dektech.com.au; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1 of 1] log: fix errors reported by cppcheck version
1.75 [#1985]

Hi Canh Van Truong,

On 8/31/2016 12:04 PM, Canh Van Truong wrote:
>   sprintf(command, VERIFY_CMD_, appLogPath,
> - "safLgStrCfg=verDefaultLogFileFmt", tZoneMillP);
> + "safLgStr=verDefaultLogFileFmt",tZoneMillP);
[AVM] why string changed , what was the errors, reported by cppcheck ?
[Canh]  No, this is not reported by cppcheck.
In the test case, we created runtime stream, object with command:
(saflogger -a ...).
And RDN to create runtime object is  'safLgStr=' , not is '
safLgStrCfg='. 

On 8/31/2016 12:04 PM, Canh Van Truong wrote:
>   osaf/services/saf/logsv/lgs/lgs_clm.cc |  2 +-
>   osaf/services/saf/logsv/lgs/lgs_evt.cc |  3 ++-
>   tests/logsv/tet_LogOiOps.c |  4 ++--
>   tests/logsv/tet_log_longDN.c   |  2 +-
>   4 files changed, 6 insertions(+), 5 deletions(-)
>
>
> Fix errors, reported by cppcheck:
>
> osaf/services/saf/logsv/lgs/lgs_clm.cc:120]: (error) Uninitialized 
> variable: rc
> osaf/services/saf/logsv/lgs/lgs_evt.cc:892]: (error) Invalid strncmp()
argument nr 3. A non-boolean value is required.
>
> diff --git a/osaf/services/saf/logsv/lgs/lgs_clm.cc 
> b/osaf/services/saf/logsv/lgs/lgs_clm.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_clm.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_clm.cc
> @@ -93,7 +93,7 @@ static uint32_t lgs_clm_node_find(NODE_I
>*/
>   static uint32_t lgs_clm_node_add(NODE_ID clm_node_id) {
> TRACE_ENTER();
> -  uint32_t rc;
> +  uint32_t rc = NCSCC_RC_SUCCESS;
> lgs_clm_node_t *clm_node;
>   
> clm_node = new lgs_clm_node_t();
> diff --git a/osaf/services/saf/logsv/lgs/lgs_evt.cc 
> b/osaf/services/saf/logsv/lgs/lgs_evt.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_evt.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_evt.cc
> @@ -799,6 +799,7 @@ SaAisErrorT create_new_app_stream(lgsv_s
> SaUint32T logMaxLogrecsize_conf = 0;
> SaConstStringT str_name;
> int num, err = 0;
> +  const char *dnPrefix = "safLgStr=";
>   
> TRACE_ENTER();
>   
> @@ -873,7 +874,7 @@ SaAisErrorT create_new_app_stream(lgsv_s
>   
> /* Verify that the name seems to be a DN */
> str_name = osaf_extended_name_borrow(&open_sync_param->lstr_name);
> -  if (strncmp("safLgStr=", str_name, sizeof("safLgStr=") != 0)) {
> +  if (strncmp(dnPrefix, str_name, strlen(dnPrefix)) != 0) {
>   TRACE("'%s' is not a valid stream name => invalid param", str_name);
>   rc = SA_AIS_ERR_INVALID_PARAM;
>   goto done;
> diff --git a/tests/logsv/tet_LogOiOps.c b/tests/logsv/tet_LogOiOps.c
> --- a/tests/logsv/tet_LogOiOps.c
> +++ b/tests/logsv/tet_LogOiOps.c
> @@ -2443,7 +2443,7 @@ void verDefaultLogFileFmt(void)
>   rc = system(command);
>   if (WEXITSTATUS(rc) == 0) {
>   /* Send an system log to app stream */
> - rc = system("saflogger -a
safLgStrCfg=verDefaultLogFileFmt");
> + rc = system("saflogger -a safLgStr=verDefaultLogFileFmt");
>   if (WEXITSTATUS(rc)) {
>   /* Failed to send log record to app stream */
>   fprintf(stderr, "Failed to invoke saflogger -a\n");
@@ -2457,7 
> +2457,7 @@ void verDefaultLogFileFmt(void)
>   /* Verify the content of log file if it is reflected with
right format */
>   sprintf(appLogPath, "%s/saflogger", log_root_path);
>   sprintf(command, VERIFY_CMD_, appLogPath,
> - "safLgStrCfg=verDefaultLogFileFmt", tZoneMillP);
> + "safLgStr=verDefaultLogFileFmt",tZoneMillP);
>   
>   rc = system(command);
>   if (rc == -1) {
> diff --git a/tests/logsv/tet_log_longDN.c 
> b/tests/logsv/tet_log_longDN.c
> --- a/tests/logsv/tet_log_longDN.c
> +++ b/tests/logsv/tet_log_longDN.c
> @@ -905,7 +905,7 @@ void longDNIn_AppStreamDN(void)
>   memset(appStreamDN, 'D', sizeof(appStreamDN) - 1);
>   
>   // Perform testing
> - sprintf(command, "saflogger -a safLgStrCfg=%s -f longDN
longDN_test", appStreamDN);
> + sprintf(command, "saflogger -a safLgStr=%s -f longDN longDN_test", 
> +appStreamDN);
>   rc = system(command);
>   if (WEXITSTATUS(rc) != 0) {
>   fprintf(stderr, "Failed to perform cmd = %s\n", command);



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


Re: [devel] [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]

2016-08-31 Thread A V Mahesh
Hi HansN,

 >> I have not tested this

Ok Thanks for the tips, I will check partially TIPC_DEST_DROPPABLE 
enabled & disabled case
and then we can conclude .

-AVM

On 9/1/2016 10:59 AM, Hans Nordebäck wrote:
> Hi Mahesh,
>
> I have not tested this, but the following should work:
>
> - Set BSRsock TIPC_IMPORTANCE to TIPC_LOW_IMPORTANCE
>
> - set socket receive buffer to a small value:
>
>   optval = "small socket recieive buffer size" , 5000 ?
>
>   setsockopt(tipc_cb.BSRsock, SOL_SOCKET, SO_RCVBUF, &optval, optlen)
>
> -  sysctl -w net.tipc.tipc_rmem="5000 4000 68240400" (or smaller 
> values)
>
> - add some delays when processing messages in 
> mdtm_process_recv_events(), to provoke overloading the socket receive 
> buffer.
>
> We experience dropped packages in a 75 node system, and as a 
> workaround increasing the default so receive buffer size it seems 
> working for that setup.
>
> /Thanks HansN
>
> On 09/01/2016 05:50 AM, A V Mahesh wrote:
>> Hi HansN,
>>
>> Do you have any tips to created overload case,
>>
>> I would like test and observe TIPC_DEST_DROPPABLE enabled & disabled 
>> cases.
>>
>> -AVM
>>
>>
>> On 9/1/2016 9:12 AM, A V Mahesh wrote:
>>> Hi HansN,
>>>
>>> Sorry for the delay.
>>>
>>> I will test it and get back to you soon.
>>>
>>> -AVM
>>>
>>>
>>> On 8/31/2016 4:29 PM, Hans Nordebäck wrote:
 Hi Mahesh,
 Any updates on this?

 /Regards HansN

 -Original Message-
 From: Anders Widell
 Sent: den 25 augusti 2016 13:11
 To: A V Mahesh ; Hans Nordebäck 
 ; mathi.naic...@oracle.com
 Cc: opensaf-devel@lists.sourceforge.net
 Subject: Re: [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]

 Hi!

 This is what the TIPC user documentation says about 
 TIPC_DEST_DROPPABLE:
 "This option governs the handling of messages sent by the socket if 
 the message cannot be delivered to its destination, either because 
 the receiver is congested or because the specified receiver does 
 not exist.
 If enabled, the message is discarded; otherwise the message is 
 returned to the sender."

 This is what the TIPC user documentation says about the return 
 value from the recvmsg() system call: "When used with a 
 connectionless socket, a return value of 0 indicates the arrival of 
 a returned data message that was originally sent by this socket."

 I think the documentation is pretty clear. If you set 
 TIPC_DEST_DROPPABLE to true, the receiver can discard messages e.g. 
 when the receive buffer is full. The sender will not be notified in 
 this case. If TIPC_DEST_DROPPABLE is set to false, the message will 
 be returned to the sender in case of a full receive buffer. The 
 sender knows that it has received such a returned message when the 
 recvmsg() call returns zero.

 regards,
 Anders Widell

 On 08/25/2016 11:30 AM, A V Mahesh wrote:
> Hi HansN,
>
>
> On 8/23/2016 5:22 PM, Hans Nordebäck wrote:
>
>> Hi Mahesh,
>>
>> Yes, this is my understanding too, if TIPC_DROPPABLE = true tipc may
>> drop messages silently,  at receive sock buffer full condition,  but
>> do not return any ancillary message.
>> If TIPC_DROPPABLE = false tipc may drop message but will send an
>> ancillary message to inform about TIPC_ERR_OVERLOAD.
> [AVM]
>
> My observation are understanding is different, based on TIPC code and
> Linux TIPC 2.0 Programmer's Guide , that the TIPC_ERR_OVERLOAD error
> returned when TIPC is unable to enqueue an incoming message on the
> receiving socket's receive queue irrelevant of TIPC_DEST_DROPPABLE
> enabled or disabled.
>
> The only difference between TIPC_DEST_DROPPABLE enabled or 
> disabled is
> , If  TIPC_DEST_DROPPABLE enabled, the message is discarded and
> recvmsg() returned size is ZERO and application will get errors, if
> TIPC_DEST_DROPPABLE disabled  the message is returned to the 
> sender it
> means the recvmsg() returned size is user send data size and
> application will get errors .
>
> I did check the TIPC code and documentations  and I haven't get any
> evidences that  TIPC_ERR_OVERLOAD error code will be send only If
> TIPC_DEST_DROPPABLE = false.
>
> Even while testing #1227
> (https://sourceforge.net/p/opensaf/mailman/message/33207717/) my
> observations and understanding was, an individual TIPC socket is only
> allowed to queue up
> OVERLOAD_LIMIT_BASE/2 messages of the lowest importance level before
> it starts rejecting them.
> Once a socket receiving queue length exceeds the maximum limit value,
> the receiving socket will send out a reject message  with
> TIPC_ERR_OVERLOAD error code with cmsg_type as
> TIPC_ERRINFO/TIPC_RETDATA, and the tipc code and Linux TIPC 2.0
> Programmer's Guide  confirmed the same .
>
> tipc/socket.

Re: [devel] [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]

2016-08-31 Thread Hans Nordebäck
Hi Mahesh,

I have not tested this, but the following should work:

- Set BSRsock TIPC_IMPORTANCE to TIPC_LOW_IMPORTANCE

- set socket receive buffer to a small value:

   optval = "small socket recieive buffer size" , 5000 ?

   setsockopt(tipc_cb.BSRsock, SOL_SOCKET, SO_RCVBUF, &optval, optlen)

-  sysctl -w net.tipc.tipc_rmem="5000 4000 68240400" (or smaller values)

- add some delays when processing messages in 
mdtm_process_recv_events(), to provoke overloading the socket receive 
buffer.

We experience dropped packages in a 75 node system, and as a workaround 
increasing the default so receive buffer size it seems working for that 
setup.

/Thanks HansN

On 09/01/2016 05:50 AM, A V Mahesh wrote:
> Hi HansN,
>
> Do you have any tips to created overload case,
>
> I would like test and observe TIPC_DEST_DROPPABLE enabled & disabled 
> cases.
>
> -AVM
>
>
> On 9/1/2016 9:12 AM, A V Mahesh wrote:
>> Hi HansN,
>>
>> Sorry for the delay.
>>
>> I will test it and get back to you soon.
>>
>> -AVM
>>
>>
>> On 8/31/2016 4:29 PM, Hans Nordebäck wrote:
>>> Hi Mahesh,
>>> Any updates on this?
>>>
>>> /Regards HansN
>>>
>>> -Original Message-
>>> From: Anders Widell
>>> Sent: den 25 augusti 2016 13:11
>>> To: A V Mahesh ; Hans Nordebäck 
>>> ; mathi.naic...@oracle.com
>>> Cc: opensaf-devel@lists.sourceforge.net
>>> Subject: Re: [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]
>>>
>>> Hi!
>>>
>>> This is what the TIPC user documentation says about 
>>> TIPC_DEST_DROPPABLE:
>>> "This option governs the handling of messages sent by the socket if 
>>> the message cannot be delivered to its destination, either because 
>>> the receiver is congested or because the specified receiver does not 
>>> exist.
>>> If enabled, the message is discarded; otherwise the message is 
>>> returned to the sender."
>>>
>>> This is what the TIPC user documentation says about the return value 
>>> from the recvmsg() system call: "When used with a connectionless 
>>> socket, a return value of 0 indicates the arrival of a returned data 
>>> message that was originally sent by this socket."
>>>
>>> I think the documentation is pretty clear. If you set 
>>> TIPC_DEST_DROPPABLE to true, the receiver can discard messages e.g. 
>>> when the receive buffer is full. The sender will not be notified in 
>>> this case. If TIPC_DEST_DROPPABLE is set to false, the message will 
>>> be returned to the sender in case of a full receive buffer. The 
>>> sender knows that it has received such a returned message when the 
>>> recvmsg() call returns zero.
>>>
>>> regards,
>>> Anders Widell
>>>
>>> On 08/25/2016 11:30 AM, A V Mahesh wrote:
 Hi HansN,


 On 8/23/2016 5:22 PM, Hans Nordebäck wrote:

> Hi Mahesh,
>
> Yes, this is my understanding too, if TIPC_DROPPABLE = true tipc may
> drop messages silently,  at receive sock buffer full condition,  but
> do not return any ancillary message.
> If TIPC_DROPPABLE = false tipc may drop message but will send an
> ancillary message to inform about TIPC_ERR_OVERLOAD.
 [AVM]

 My observation are understanding is different, based on TIPC code and
 Linux TIPC 2.0 Programmer's Guide , that the TIPC_ERR_OVERLOAD error
 returned when TIPC is unable to enqueue an incoming message on the
 receiving socket's receive queue irrelevant of TIPC_DEST_DROPPABLE
 enabled or disabled.

 The only difference between TIPC_DEST_DROPPABLE enabled or disabled is
 , If  TIPC_DEST_DROPPABLE enabled, the message is discarded and
 recvmsg() returned size is ZERO and application will get errors, if
 TIPC_DEST_DROPPABLE disabled  the message is returned to the sender it
 means the recvmsg() returned size is user send data size and
 application will get errors .

 I did check the TIPC code and documentations  and I haven't get any
 evidences that  TIPC_ERR_OVERLOAD error code will be send only If
 TIPC_DEST_DROPPABLE = false.

 Even while testing #1227
 (https://sourceforge.net/p/opensaf/mailman/message/33207717/) my
 observations and understanding was, an individual TIPC socket is only
 allowed to queue up
 OVERLOAD_LIMIT_BASE/2 messages of the lowest importance level before
 it starts rejecting them.
 Once a socket receiving queue length exceeds the maximum limit value,
 the receiving socket will send out a reject message  with
 TIPC_ERR_OVERLOAD error code with cmsg_type as
 TIPC_ERRINFO/TIPC_RETDATA, and the tipc code and Linux TIPC 2.0
 Programmer's Guide  confirmed the same .

 tipc/socket.c
 ===
 /* Reject message if there isn't room to queue it */

 recv_q_len = (u32)atomic_read(&tipc_queue_size);
 if (unlikely(recv_q_len >= OVERLOAD_LIMIT_BASE)) {
  if (rx_queue_full(msg, recv_q_len, OVERLOAD_LIMIT_BASE))
  return TIPC_ERR_OVERLOAD;
 }
 recv_q_len = sk

[devel] Immnd: maximum Ccbs limit 10000 has been reached very quickly

2016-08-31 Thread A V Mahesh
Hi All,

I was running  `immcfg` in a loop to create some object , once it 
reaches 10001objects creation
Immnd is returning `ERR_NO_RESOURCES: maximum Ccbs limit 1 has been 
reached for the cluster` error
which is unexpected ,  once `immcfg`  reruns , it is expected that the  
`maximum Ccbs` will decremented ( this was previous behavior) ,

can you please check.

=
 


for (( i = 1 ; i <=30; i++))
immcfg -c PinvId -a pinvPhoneNumber=+46768 pinvRdn=$i

Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO Ccb 9996 COMMITTED 
(immcfg_SC-1_2334)
Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO Ccb 9997 COMMITTED 
(immcfg_SC-1_2337)
Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO Ccb 9998 COMMITTED 
(immcfg_SC-1_2340)
Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO Ccb  COMMITTED 
(immcfg_SC-1_2343)
Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO Ccb 1 COMMITTED 
(immcfg_SC-1_2346)
Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO Ccb 10001 COMMITTED 
(immcfg_SC-1_2349)
Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO ERR_NO_RESOURCES: maximum Ccbs 
limit 1 has been reached for the cluster
Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO ERR_NO_RESOURCES: maximum Ccbs 
limit 1 has been reached for the cluster
Sep  1 10:43:33 SC-1 osafimmnd[4466]: NO ERR_NO_RESOURCES: maximum Ccbs 
limit 1 has been reached for the cluster


=
 


-AVM


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


Re: [devel] [PATCH 1 of 1] log: fix cppcheck with performance severity [#1975]

2016-08-31 Thread A V Mahesh
  Hi Canh Van Truong,

  Can you please share the suggestion reported by cppcheck.

-AVM


On 8/31/2016 2:19 PM, Canh Van Truong wrote:
>   osaf/services/saf/logsv/lgs/lgs_config.cc |  35 
> --
>   osaf/services/saf/logsv/lgs/lgs_file.cc   |   4 +-
>   2 files changed, 16 insertions(+), 23 deletions(-)
>
>
> Fix performance reported by cppcheck.
>
> diff --git a/osaf/services/saf/logsv/lgs/lgs_config.cc 
> b/osaf/services/saf/logsv/lgs/lgs_config.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_config.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_config.cc
> @@ -137,32 +137,25 @@ typedef struct _lgs_conf_t {
> lgs_conf_flg_t logDataGroupname_cnfflag;
> lgs_conf_flg_t logStreamFileFormat_cnfflag;
>   
> -  _lgs_conf_t() {
> -/*
> - * For the following flags, LGS_CNF_DEF means that no external
> - * configuration exists and the corresponding attributes hard-coded
> - * default value is used.Is set to false if configuration is found in
> - * IMM object or environment variable.
> - * See function lgs_logconf_get() for more info.
> - */
> +  _lgs_conf_t()
> +  : logRootDirectory(lgs_conf_def.logRootDirectory),
> +logRootDirectory_cnfflag(LGS_CNF_DEF),
> +logMaxLogrecsize_cnfflag(LGS_CNF_DEF),
> +logStreamSystemHighLimit_cnfflag(LGS_CNF_DEF),
> +logStreamSystemLowLimit_cnfflag(LGS_CNF_DEF),
> +logStreamAppHighLimit_cnfflag(LGS_CNF_DEF),
> +logStreamAppLowLimit_cnfflag(LGS_CNF_DEF),
> +logMaxApplicationStreams_cnfflag(LGS_CNF_DEF),
> +logFileIoTimeout_cnfflag(LGS_CNF_DEF),
> +logFileSysConfig_cnfflag(LGS_CNF_DEF),
> +logDataGroupname_cnfflag(LGS_CNF_DEF),
> +logStreamFileFormat_cnfflag(LGS_CNF_DEF)
> +  {
>   OpenSafLogConfig_object_exist = false;
> -logRootDirectory_cnfflag = LGS_CNF_DEF;
> -logStreamSystemHighLimit_cnfflag = LGS_CNF_DEF;
> -logStreamSystemLowLimit_cnfflag = LGS_CNF_DEF;
> -logStreamAppHighLimit_cnfflag = LGS_CNF_DEF;
> -logStreamAppLowLimit_cnfflag = LGS_CNF_DEF;
> -logDataGroupname_cnfflag = LGS_CNF_DEF;
>   /*
>* The following attributes cannot be configured in the config file
>* Will be set to false if the attribute exists in the IMM config object
>*/
> -logMaxLogrecsize_cnfflag = LGS_CNF_DEF;
> -logMaxApplicationStreams_cnfflag = LGS_CNF_DEF;
> -logFileIoTimeout_cnfflag = LGS_CNF_DEF;
> -logFileSysConfig_cnfflag = LGS_CNF_DEF;
> -logStreamFileFormat_cnfflag = LGS_CNF_DEF;
> -
> -logRootDirectory = lgs_conf_def.logRootDirectory;
>   (void) strcpy(logDataGroupname, lgs_conf_def.logDataGroupname);
>   (void) strcpy(logStreamFileFormat, lgs_conf_def.logStreamFileFormat);
>   logMaxLogrecsize = lgs_conf_def.logMaxLogrecsize;
> diff --git a/osaf/services/saf/logsv/lgs/lgs_file.cc 
> b/osaf/services/saf/logsv/lgs/lgs_file.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_file.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_file.cc
> @@ -51,11 +51,11 @@ struct file_communicate {
> size_t outdata_size;
> void *outdata_ptr;  /* Out data from handlers */
>   
> -  file_communicate() {
> +  file_communicate() : request_code(LGSF_NOREQ)
> +  {
>   answer_f = false;
>   request_f = false;
>   timeout_f = false;
> -request_code = LGSF_NOREQ;
>   return_code = LGSF_NORETC;
>   indata_ptr = NULL;
>   outdata_ptr = NULL;


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


Re: [devel] [PATCH 1 of 1] log: fix errors reported by cppcheck version 1.75 [#1985]

2016-08-31 Thread A V Mahesh
Hi Canh Van Truong,

On 8/31/2016 12:04 PM, Canh Van Truong wrote:
>   sprintf(command, VERIFY_CMD_, appLogPath,
> - "safLgStrCfg=verDefaultLogFileFmt", tZoneMillP);
> + "safLgStr=verDefaultLogFileFmt",tZoneMillP);
[AVM] why string changed , what was the errors, reported by cppcheck ?

On 8/31/2016 12:04 PM, Canh Van Truong wrote:
>   osaf/services/saf/logsv/lgs/lgs_clm.cc |  2 +-
>   osaf/services/saf/logsv/lgs/lgs_evt.cc |  3 ++-
>   tests/logsv/tet_LogOiOps.c |  4 ++--
>   tests/logsv/tet_log_longDN.c   |  2 +-
>   4 files changed, 6 insertions(+), 5 deletions(-)
>
>
> Fix errors, reported by cppcheck:
>
> osaf/services/saf/logsv/lgs/lgs_clm.cc:120]: (error) Uninitialized variable: 
> rc
> osaf/services/saf/logsv/lgs/lgs_evt.cc:892]: (error) Invalid strncmp() 
> argument nr 3. A non-boolean value is required.
>
> diff --git a/osaf/services/saf/logsv/lgs/lgs_clm.cc 
> b/osaf/services/saf/logsv/lgs/lgs_clm.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_clm.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_clm.cc
> @@ -93,7 +93,7 @@ static uint32_t lgs_clm_node_find(NODE_I
>*/
>   static uint32_t lgs_clm_node_add(NODE_ID clm_node_id) {
> TRACE_ENTER();
> -  uint32_t rc;
> +  uint32_t rc = NCSCC_RC_SUCCESS;
> lgs_clm_node_t *clm_node;
>   
> clm_node = new lgs_clm_node_t();
> diff --git a/osaf/services/saf/logsv/lgs/lgs_evt.cc 
> b/osaf/services/saf/logsv/lgs/lgs_evt.cc
> --- a/osaf/services/saf/logsv/lgs/lgs_evt.cc
> +++ b/osaf/services/saf/logsv/lgs/lgs_evt.cc
> @@ -799,6 +799,7 @@ SaAisErrorT create_new_app_stream(lgsv_s
> SaUint32T logMaxLogrecsize_conf = 0;
> SaConstStringT str_name;
> int num, err = 0;
> +  const char *dnPrefix = "safLgStr=";
>   
> TRACE_ENTER();
>   
> @@ -873,7 +874,7 @@ SaAisErrorT create_new_app_stream(lgsv_s
>   
> /* Verify that the name seems to be a DN */
> str_name = osaf_extended_name_borrow(&open_sync_param->lstr_name);
> -  if (strncmp("safLgStr=", str_name, sizeof("safLgStr=") != 0)) {
> +  if (strncmp(dnPrefix, str_name, strlen(dnPrefix)) != 0) {
>   TRACE("'%s' is not a valid stream name => invalid param", str_name);
>   rc = SA_AIS_ERR_INVALID_PARAM;
>   goto done;
> diff --git a/tests/logsv/tet_LogOiOps.c b/tests/logsv/tet_LogOiOps.c
> --- a/tests/logsv/tet_LogOiOps.c
> +++ b/tests/logsv/tet_LogOiOps.c
> @@ -2443,7 +2443,7 @@ void verDefaultLogFileFmt(void)
>   rc = system(command);
>   if (WEXITSTATUS(rc) == 0) {
>   /* Send an system log to app stream */
> - rc = system("saflogger -a safLgStrCfg=verDefaultLogFileFmt");
> + rc = system("saflogger -a safLgStr=verDefaultLogFileFmt");
>   if (WEXITSTATUS(rc)) {
>   /* Failed to send log record to app stream */
>   fprintf(stderr, "Failed to invoke saflogger -a\n");
> @@ -2457,7 +2457,7 @@ void verDefaultLogFileFmt(void)
>   /* Verify the content of log file if it is reflected with right 
> format */
>   sprintf(appLogPath, "%s/saflogger", log_root_path);
>   sprintf(command, VERIFY_CMD_, appLogPath,
> - "safLgStrCfg=verDefaultLogFileFmt", tZoneMillP);
> + "safLgStr=verDefaultLogFileFmt",tZoneMillP);
>   
>   rc = system(command);
>   if (rc == -1) {
> diff --git a/tests/logsv/tet_log_longDN.c b/tests/logsv/tet_log_longDN.c
> --- a/tests/logsv/tet_log_longDN.c
> +++ b/tests/logsv/tet_log_longDN.c
> @@ -905,7 +905,7 @@ void longDNIn_AppStreamDN(void)
>   memset(appStreamDN, 'D', sizeof(appStreamDN) - 1);
>   
>   // Perform testing
> - sprintf(command, "saflogger -a safLgStrCfg=%s -f longDN longDN_test", 
> appStreamDN);
> + sprintf(command, "saflogger -a safLgStr=%s -f longDN longDN_test", 
> appStreamDN);
>   rc = system(command);
>   if (WEXITSTATUS(rc) != 0) {
>   fprintf(stderr, "Failed to perform cmd = %s\n", command);


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


Re: [devel] [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]

2016-08-31 Thread A V Mahesh
Hi HansN,

Do you have any tips to created overload case,

I would like test and observe TIPC_DEST_DROPPABLE enabled & disabled cases.

-AVM


On 9/1/2016 9:12 AM, A V Mahesh wrote:
> Hi HansN,
>
> Sorry for the delay.
>
> I will test it and get back to you soon.
>
> -AVM
>
>
> On 8/31/2016 4:29 PM, Hans Nordebäck wrote:
>> Hi Mahesh,
>> Any updates on this?
>>
>> /Regards HansN
>>
>> -Original Message-
>> From: Anders Widell
>> Sent: den 25 augusti 2016 13:11
>> To: A V Mahesh ; Hans Nordebäck 
>> ; mathi.naic...@oracle.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]
>>
>> Hi!
>>
>> This is what the TIPC user documentation says about TIPC_DEST_DROPPABLE:
>> "This option governs the handling of messages sent by the socket if 
>> the message cannot be delivered to its destination, either because 
>> the receiver is congested or because the specified receiver does not 
>> exist.
>> If enabled, the message is discarded; otherwise the message is 
>> returned to the sender."
>>
>> This is what the TIPC user documentation says about the return value 
>> from the recvmsg() system call: "When used with a connectionless 
>> socket, a return value of 0 indicates the arrival of a returned data 
>> message that was originally sent by this socket."
>>
>> I think the documentation is pretty clear. If you set 
>> TIPC_DEST_DROPPABLE to true, the receiver can discard messages e.g. 
>> when the receive buffer is full. The sender will not be notified in 
>> this case. If TIPC_DEST_DROPPABLE is set to false, the message will 
>> be returned to the sender in case of a full receive buffer. The 
>> sender knows that it has received such a returned message when the 
>> recvmsg() call returns zero.
>>
>> regards,
>> Anders Widell
>>
>> On 08/25/2016 11:30 AM, A V Mahesh wrote:
>>> Hi HansN,
>>>
>>>
>>> On 8/23/2016 5:22 PM, Hans Nordebäck wrote:
>>>
 Hi Mahesh,

 Yes, this is my understanding too, if TIPC_DROPPABLE = true tipc may
 drop messages silently,  at receive sock buffer full condition,  but
 do not return any ancillary message.
 If TIPC_DROPPABLE = false tipc may drop message but will send an
 ancillary message to inform about TIPC_ERR_OVERLOAD.
>>> [AVM]
>>>
>>> My observation are understanding is different, based on TIPC code and
>>> Linux TIPC 2.0 Programmer's Guide , that the TIPC_ERR_OVERLOAD error
>>> returned when TIPC is unable to enqueue an incoming message on the
>>> receiving socket's receive queue irrelevant of TIPC_DEST_DROPPABLE
>>> enabled or disabled.
>>>
>>> The only difference between TIPC_DEST_DROPPABLE enabled or disabled is
>>> , If  TIPC_DEST_DROPPABLE enabled, the message is discarded and
>>> recvmsg() returned size is ZERO and application will get errors, if
>>> TIPC_DEST_DROPPABLE disabled  the message is returned to the sender it
>>> means the recvmsg() returned size is user send data size and
>>> application will get errors .
>>>
>>> I did check the TIPC code and documentations  and I haven't get any
>>> evidences that  TIPC_ERR_OVERLOAD error code will be send only If
>>> TIPC_DEST_DROPPABLE = false.
>>>
>>> Even while testing #1227
>>> (https://sourceforge.net/p/opensaf/mailman/message/33207717/) my
>>> observations and understanding was, an individual TIPC socket is only
>>> allowed to queue up
>>> OVERLOAD_LIMIT_BASE/2 messages of the lowest importance level before
>>> it starts rejecting them.
>>> Once a socket receiving queue length exceeds the maximum limit value,
>>> the receiving socket will send out a reject message  with
>>> TIPC_ERR_OVERLOAD error code with cmsg_type as
>>> TIPC_ERRINFO/TIPC_RETDATA, and the tipc code and Linux TIPC 2.0
>>> Programmer's Guide  confirmed the same .
>>>
>>> tipc/socket.c
>>> ===
>>> /* Reject message if there isn't room to queue it */
>>>
>>> recv_q_len = (u32)atomic_read(&tipc_queue_size);
>>> if (unlikely(recv_q_len >= OVERLOAD_LIMIT_BASE)) {
>>>  if (rx_queue_full(msg, recv_q_len, OVERLOAD_LIMIT_BASE))
>>>  return TIPC_ERR_OVERLOAD;
>>> }
>>> recv_q_len = skb_queue_len(&sk->sk_receive_queue);
>>> if (unlikely(recv_q_len >= (OVERLOAD_LIMIT_BASE / 2))) {
>>>  if (rx_queue_full(msg, recv_q_len, OVERLOAD_LIMIT_BASE / 2))
>>>  return TIPC_ERR_OVERLOAD;
>>> }
>>> ===
>>>
>>>
>>> 2.1.17. setsockopt() of  TIPC 2.0 Programmer's Guide
>>> ===
>>> TIPC_DEST_DROPPABLE
>>> This option governs the handling of messages sent by the socket if the
>>> message cannot be delivered to its destination, either because the
>>> receiver is congested or because the specified receiver does not
>>> exist. If enabled, the message is discarded; otherwise the message is
>>> returned to the sender.
>>>
>>> By default, this option is disabled for SOCK_SEQPACKET and SOCK_STREAM
>>> socket types, and en

Re: [devel] [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]

2016-08-31 Thread A V Mahesh
Hi HansN,

Sorry for the delay.

I will test it and get back to you soon.

-AVM


On 8/31/2016 4:29 PM, Hans Nordebäck wrote:
> Hi Mahesh,
> Any updates on this?
>
> /Regards HansN
>
> -Original Message-
> From: Anders Widell
> Sent: den 25 augusti 2016 13:11
> To: A V Mahesh ; Hans Nordebäck 
> ; mathi.naic...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]
>
> Hi!
>
> This is what the TIPC user documentation says about TIPC_DEST_DROPPABLE:
> "This option governs the handling of messages sent by the socket if the 
> message cannot be delivered to its destination, either because the receiver 
> is congested or because the specified receiver does not exist.
> If enabled, the message is discarded; otherwise the message is returned to 
> the sender."
>
> This is what the TIPC user documentation says about the return value from the 
> recvmsg() system call: "When used with a connectionless socket, a return 
> value of 0 indicates the arrival of a returned data message that was 
> originally sent by this socket."
>
> I think the documentation is pretty clear. If you set TIPC_DEST_DROPPABLE to 
> true, the receiver can discard messages e.g. when the receive buffer is full. 
> The sender will not be notified in this case. If TIPC_DEST_DROPPABLE is set 
> to false, the message will be returned to the sender in case of a full 
> receive buffer. The sender knows that it has received such a returned message 
> when the recvmsg() call returns zero.
>
> regards,
> Anders Widell
>
> On 08/25/2016 11:30 AM, A V Mahesh wrote:
>> Hi HansN,
>>
>>
>> On 8/23/2016 5:22 PM, Hans Nordebäck wrote:
>>
>>> Hi Mahesh,
>>>
>>> Yes, this is my understanding too, if TIPC_DROPPABLE = true tipc may
>>> drop messages silently,  at receive sock buffer full condition,  but
>>> do not return any ancillary message.
>>> If TIPC_DROPPABLE = false tipc may drop message but will send an
>>> ancillary message to inform about TIPC_ERR_OVERLOAD.
>> [AVM]
>>
>> My observation are understanding is different, based on TIPC code and
>> Linux TIPC 2.0 Programmer's Guide , that the TIPC_ERR_OVERLOAD error
>> returned when TIPC is unable to enqueue an incoming message on the
>> receiving socket's receive queue irrelevant of TIPC_DEST_DROPPABLE
>> enabled or disabled.
>>
>> The only difference between TIPC_DEST_DROPPABLE enabled or disabled is
>> , If  TIPC_DEST_DROPPABLE enabled, the message is discarded and
>> recvmsg() returned size is ZERO and application will get errors, if
>> TIPC_DEST_DROPPABLE disabled  the message is returned to the sender it
>> means the recvmsg() returned size is user send data size and
>> application will get errors .
>>
>> I did check the TIPC code and documentations  and I haven't  get any
>> evidences that  TIPC_ERR_OVERLOAD error code will be send only If
>> TIPC_DEST_DROPPABLE = false.
>>
>> Even while testing #1227
>> (https://sourceforge.net/p/opensaf/mailman/message/33207717/) my
>> observations and understanding was, an individual TIPC socket is only
>> allowed to queue up
>> OVERLOAD_LIMIT_BASE/2 messages of the lowest importance level before
>> it starts rejecting them.
>> Once a socket receiving queue length exceeds the maximum limit value,
>> the receiving socket will send out a reject message  with
>> TIPC_ERR_OVERLOAD error code with cmsg_type as
>> TIPC_ERRINFO/TIPC_RETDATA, and the tipc code and Linux TIPC 2.0
>> Programmer's Guide  confirmed the same .
>>
>> tipc/socket.c
>> ===
>> /* Reject message if there isn't room to queue it */
>>
>> recv_q_len = (u32)atomic_read(&tipc_queue_size);
>> if (unlikely(recv_q_len >= OVERLOAD_LIMIT_BASE)) {
>>  if (rx_queue_full(msg, recv_q_len, OVERLOAD_LIMIT_BASE))
>>  return TIPC_ERR_OVERLOAD;
>> }
>> recv_q_len = skb_queue_len(&sk->sk_receive_queue);
>> if (unlikely(recv_q_len >= (OVERLOAD_LIMIT_BASE / 2))) {
>>  if (rx_queue_full(msg, recv_q_len, OVERLOAD_LIMIT_BASE / 2))
>>  return TIPC_ERR_OVERLOAD;
>> }
>> ===
>>
>>
>> 2.1.17. setsockopt() of  TIPC 2.0 Programmer's Guide
>> ===
>> TIPC_DEST_DROPPABLE
>> This option governs the handling of messages sent by the socket if the
>> message cannot be delivered to its destination, either because the
>> receiver is congested or because the specified receiver does not
>> exist. If enabled, the message is discarded; otherwise the message is
>> returned to the sender.
>>
>> By default, this option is disabled for SOCK_SEQPACKET and SOCK_STREAM
>> socket types, and enabled for SOCK_RDM and SOCK_DGRAM, This
>> arrangement ensures proper teardown of failed connections when
>> connection-oriented data transfer is used, without increasing the
>> complexity of connectionless data transfer.
>>
>> TIPC_SRC_DROPPABLE
>> This option governs the handling of messages sent by the socket

Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build failed

2016-08-31 Thread A V Mahesh
Hi Zoran,

 >>  Mahesh, what kind of comment did you mean ?

The below  HansN comment :

> [HansN]
> >I know this is test code but side effects in asserts is to be avoided, but, 
> >shouldn't the code be like:
> >rc = system(command);
> >int status = WEXITSTATUS(rc);
> >osaf_assert(status != SA_AIS_OK);  ?
> >
-AVM

On 8/31/2016 4:15 PM, Zoran Milinkovic wrote:
> Hi,
>
> I completely missed this email thread and the patch.
> I build OpenSAF with gcc 4.8.4 on Ubuntu, and haven’t had any building 
> problem.
>
> If the patch works for Anders (gcc 5.x and/or 6.x), then I'm ok with the 
> patch. I cannot test the build in my environment.
>
> Mahesh, what kind of comment did you mean ?
>
> Thanks,
> Zoran
>
> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: den 30 augusti 2016 10:40
> To: Hans Nordebäck; Zoran Milinkovic
> Cc: praveen malviya; opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build 
> failed
>
> Hi HansN,
>
> Thanks for the point , Zoran Milinkovic owner this code, jut to unblock the 
> build issue,  as workaround I pushed it , I will fw this comment to him.
>
> Zoran Milinkovic,
>
> Can you please incorporate the comment.
>
> -AVM
>
> On 8/30/2016 1:08 PM, Hans Nordebäck wrote:
>> Hi Mahesh,
>>
>> One question/comment below. /Thanks HansN
>>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: den 30 augusti 2016 08:09
>> To: praveen malviya ;
>> opensaf-devel@lists.sourceforge.net
>> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3
>> build failed
>>
>> Ok,
>>
>> I just pushed
>>
>> changeset: 7989:4d6caf6903cd
>> tag: tip
>> user:A V Mahesh 
>> date:Tue Aug 30 11:05:31 2016 +0530
>> summary: clm: test code build fix [#1906]
>>
>> -AVM
>>
>> On 8/30/2016 11:28 AM, praveen malviya wrote:
>>> Hi Mahesh,
>>>
>>> I think this patch can be pushed as a workaround patch. Any
>>> improvement on it can be taken up by CLM maintainers post FC tag.
>>>
>>> Thanks,
>>> Praveen
>>>
>>> On 30-Aug-16 11:17 AM, A V Mahesh wrote:
 Hi Zoran Milinkovic,

 If  below changes ok , we can push directly.

 
 =
 ==

 diff --git a/tests/clmsv/tet_ClmLongRdn.c
 b/tests/clmsv/tet_ClmLongRdn.c
 --- a/tests/clmsv/tet_ClmLongRdn.c
 +++ b/tests/clmsv/tet_ClmLongRdn.c
 @@ -188,19 +188,21 @@ static SaClmCallbacksT_4 clmCallback4 =
 static SaClmCallbacksT clmCallback = { nodeGetCallBack,
 clmTrackCallback };

 static void unlock_node(char *nodename) {
 +   int rc;
char command[1024];

// Unlock the node
sprintf(command, "immadm -o 1 %s", nodename);
 -   system(command);
 +   assert(rc = system(command) != -1);
>> [HansN]
>> I know this is test code but side effects in asserts is to be avoided, but, 
>> shouldn't the code be like:
>> rc = system(command);
>> int status = WEXITSTATUS(rc);
>> osaf_assert(status != SA_AIS_OK);  ?
>>
 +
 }

 static void lock_node(char *nodename) {
 +   int rc;
char command[1024];
 -
// Lock the node
sprintf(command, "immadm -o 2 %s", nodename);
 -   system(command);
 +   assert(rc = system(command) != -1);
 }

 static void remove_node(char *nodename) { @@ -209,7 +211,7 @@
 static void remove_node(char *nodename)

// Lock the node
sprintf(command, "immadm -o 2 %s", nodename);
 -   system(command);
 +   assert(rc = system(command) != -1);

// Remove the node
sprintf(command, "immcfg -d %s", nodename);
 
 =
 ==


 -AVM

 On 8/30/2016 10:46 AM, A V Mahesh wrote:
> Hi All,
>
> I just build  http://hg.code.sf.net/p/opensaf/staging with
> following changeset as sanity test
>
> it build failed with .
>
> changeset:   7982:106230d848a6
> tag: tip
> parent:  7979:aec46cc64cc8
> user:Anders Widell 
> date:Mon Aug 29 19:29:55 2016 +0200
> summary: uml: Update the UML environment [#1979]
>
> ===
> =
> ===
>
>
> /gcc -DHAVE_CONFIG_H -I. -I../.. -DSA_CLM_B01=1 -I../..
> -I../../osaf/libs/saf/include -I../../osaf/libs/core/include
> -I../../osaf/libs/core/leap/include
> -I../../osaf/libs/core/mds/include
> -I../../osaf/libs/core/common/include
> -I../../osaf/libs/core/cplusplus -I../../tests/unit_test_fw/inc
> -std=gnu11 -Wall -fno-strict-aliasing -Werror -fPIC
> -D_FORTIFY_SOURCE=2 -fstack-protec

Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build failed

2016-08-31 Thread Zoran Milinkovic
Hi,

I took a pattern with locking/unlocking nodes from other CLM tests, but I 
removed assert.
The reason for removing assert is that lock and unlock node functions are there 
only to ensure that the testing node is in a correct state. If it's not, the 
test will fail. So, assert is not needed, and will not abort CLM test.

We can create a macro with Hans proposal and use it when system() is called.

Thanks,
Zoran

-Original Message-
From: Hans Nordebäck 
Sent: den 31 augusti 2016 13:33
To: Mathivanan Naickan Palanivelu; Venkata Mahesh Alla; Zoran Milinkovic
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build failed

Hi,

I looked further down in this mail and noticed the build problem,

if a function is declared with e.g:

int foo()  __attribute__ ((__warn_unused_result__));

and called:

foo();

then compiling with -Wall

will result in the warning/error, but:

if (foo()) {}

should solve the build problem/HansN





On 08/31/2016 12:10 PM, Mathivanan Naickan Palanivelu wrote:
> Iam looking into this.
>
> Thanks,
> Mathi.
>
>> -Original Message-
>> From: A V Mahesh
>> Sent: Tuesday, August 30, 2016 2:10 PM
>> To: Hans Nordebäck; Zoran Milinkovic
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 
>> build failed
>>
>> Hi HansN,
>>
>> Thanks for the point , Zoran Milinkovic owner this code, jut to 
>> unblock the build issue,  as workaround I pushed it , I will fw this comment 
>> to him.
>>
>> Zoran Milinkovic,
>>
>> Can you please incorporate the comment.
>>
>> -AVM
>>
>> On 8/30/2016 1:08 PM, Hans Nordebäck wrote:
>>> Hi Mahesh,
>>>
>>> One question/comment below. /Thanks HansN
>>>
>>> -Original Message-
>>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>>> Sent: den 30 augusti 2016 08:09
>>> To: praveen malviya ; 
>>> opensaf-devel@lists.sourceforge.net
>>> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 
>>> build failed
>>>
>>> Ok,
>>>
>>> I just pushed
>>>
>>> changeset: 7989:4d6caf6903cd
>>> tag: tip
>>> user:A V Mahesh 
>>> date:Tue Aug 30 11:05:31 2016 +0530
>>> summary: clm: test code build fix [#1906]
>>>
>>> -AVM
>>>
>>> On 8/30/2016 11:28 AM, praveen malviya wrote:
 Hi Mahesh,

 I think this patch can be pushed as a workaround patch. Any 
 improvement on it can be taken up by CLM maintainers post FC tag.

 Thanks,
 Praveen

 On 30-Aug-16 11:17 AM, A V Mahesh wrote:
> Hi Zoran Milinkovic,
>
> If  below changes ok , we can push directly.
>
>
>> ==
>> ==
> =
> ==
>
> diff --git a/tests/clmsv/tet_ClmLongRdn.c 
> b/tests/clmsv/tet_ClmLongRdn.c
> --- a/tests/clmsv/tet_ClmLongRdn.c
> +++ b/tests/clmsv/tet_ClmLongRdn.c
> @@ -188,19 +188,21 @@ static SaClmCallbacksT_4 clmCallback4 =
> static SaClmCallbacksT clmCallback = { nodeGetCallBack, 
> clmTrackCallback };
>
> static void unlock_node(char *nodename) {
> +   int rc;
>char command[1024];
>
>// Unlock the node
>sprintf(command, "immadm -o 1 %s", nodename);
> -   system(command);
> +   assert(rc = system(command) != -1);
>>> [HansN]
>>> I know this is test code but side effects in asserts is to be 
>>> avoided, but,
>> shouldn't the code be like:
>>> rc = system(command);
>>> int status = WEXITSTATUS(rc);
>>> osaf_assert(status != SA_AIS_OK);  ?
>>>
> +
> }
>
> static void lock_node(char *nodename) {
> +   int rc;
>char command[1024];
> -
>// Lock the node
>sprintf(command, "immadm -o 2 %s", nodename);
> -   system(command);
> +   assert(rc = system(command) != -1);
> }
>
> static void remove_node(char *nodename) { @@ -209,7 +211,7 @@ 
> static void remove_node(char *nodename)
>
>// Lock the node
>sprintf(command, "immadm -o 2 %s", nodename);
> -   system(command);
> +   assert(rc = system(command) != -1);
>
>// Remove the node
>sprintf(command, "immcfg -d %s", nodename);
>
>> ==
>> ==
> =
> ==
>
>
> -AVM
>
> On 8/30/2016 10:46 AM, A V Mahesh wrote:
>> Hi All,
>>
>> I just build  http://hg.code.sf.net/p/opensaf/staging with 
>> following changeset as sanity test
>>
>> it build failed with .
>>
>> changeset:   7982:106230d848a6
>> tag: tip
>> parent:  7979:aec46cc64cc8
>> user:Anders Widell 
>> date:Mon Aug 29 19:29:55 2016 +0200
>> summary: uml: Update the UML environment [#1979]
>>
>>
>> ==

Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build failed

2016-08-31 Thread Hans Nordebäck
Hi,

I looked further down in this mail and noticed the build problem,

if a function is declared with e.g:

int foo()  __attribute__ ((__warn_unused_result__));

and called:

foo();

then compiling with -Wall

will result in the warning/error, but:

if (foo()) {}

should solve the build problem/HansN





On 08/31/2016 12:10 PM, Mathivanan Naickan Palanivelu wrote:
> Iam looking into this.
>
> Thanks,
> Mathi.
>
>> -Original Message-
>> From: A V Mahesh
>> Sent: Tuesday, August 30, 2016 2:10 PM
>> To: Hans Nordebäck; Zoran Milinkovic
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build 
>> failed
>>
>> Hi HansN,
>>
>> Thanks for the point , Zoran Milinkovic owner this code, jut to unblock the
>> build issue,  as workaround I pushed it , I will fw this comment to him.
>>
>> Zoran Milinkovic,
>>
>> Can you please incorporate the comment.
>>
>> -AVM
>>
>> On 8/30/2016 1:08 PM, Hans Nordebäck wrote:
>>> Hi Mahesh,
>>>
>>> One question/comment below. /Thanks HansN
>>>
>>> -Original Message-
>>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>>> Sent: den 30 augusti 2016 08:09
>>> To: praveen malviya ;
>>> opensaf-devel@lists.sourceforge.net
>>> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3
>>> build failed
>>>
>>> Ok,
>>>
>>> I just pushed
>>>
>>> changeset: 7989:4d6caf6903cd
>>> tag: tip
>>> user:A V Mahesh 
>>> date:Tue Aug 30 11:05:31 2016 +0530
>>> summary: clm: test code build fix [#1906]
>>>
>>> -AVM
>>>
>>> On 8/30/2016 11:28 AM, praveen malviya wrote:
 Hi Mahesh,

 I think this patch can be pushed as a workaround patch. Any
 improvement on it can be taken up by CLM maintainers post FC tag.

 Thanks,
 Praveen

 On 30-Aug-16 11:17 AM, A V Mahesh wrote:
> Hi Zoran Milinkovic,
>
> If  below changes ok , we can push directly.
>
>
>> ==
>> ==
> =
> ==
>
> diff --git a/tests/clmsv/tet_ClmLongRdn.c
> b/tests/clmsv/tet_ClmLongRdn.c
> --- a/tests/clmsv/tet_ClmLongRdn.c
> +++ b/tests/clmsv/tet_ClmLongRdn.c
> @@ -188,19 +188,21 @@ static SaClmCallbacksT_4 clmCallback4 =
> static SaClmCallbacksT clmCallback = { nodeGetCallBack,
> clmTrackCallback };
>
> static void unlock_node(char *nodename) {
> +   int rc;
>char command[1024];
>
>// Unlock the node
>sprintf(command, "immadm -o 1 %s", nodename);
> -   system(command);
> +   assert(rc = system(command) != -1);
>>> [HansN]
>>> I know this is test code but side effects in asserts is to be avoided, but,
>> shouldn't the code be like:
>>> rc = system(command);
>>> int status = WEXITSTATUS(rc);
>>> osaf_assert(status != SA_AIS_OK);  ?
>>>
> +
> }
>
> static void lock_node(char *nodename) {
> +   int rc;
>char command[1024];
> -
>// Lock the node
>sprintf(command, "immadm -o 2 %s", nodename);
> -   system(command);
> +   assert(rc = system(command) != -1);
> }
>
> static void remove_node(char *nodename) { @@ -209,7 +211,7 @@
> static void remove_node(char *nodename)
>
>// Lock the node
>sprintf(command, "immadm -o 2 %s", nodename);
> -   system(command);
> +   assert(rc = system(command) != -1);
>
>// Remove the node
>sprintf(command, "immcfg -d %s", nodename);
>
>> ==
>> ==
> =
> ==
>
>
> -AVM
>
> On 8/30/2016 10:46 AM, A V Mahesh wrote:
>> Hi All,
>>
>> I just build  http://hg.code.sf.net/p/opensaf/staging with
>> following changeset as sanity test
>>
>> it build failed with .
>>
>> changeset:   7982:106230d848a6
>> tag: tip
>> parent:  7979:aec46cc64cc8
>> user:Anders Widell 
>> date:Mon Aug 29 19:29:55 2016 +0200
>> summary: uml: Update the UML environment [#1979]
>>
>>
>> ==
>> =
>> =
>> ===
>>
>>
>> /gcc -DHAVE_CONFIG_H -I. -I../.. -DSA_CLM_B01=1 -I../..
>> -I../../osaf/libs/saf/include -I../../osaf/libs/core/include
>> -I../../osaf/libs/core/leap/include
>> -I../../osaf/libs/core/mds/include
>> -I../../osaf/libs/core/common/include
>> -I../../osaf/libs/core/cplusplus -I../../tests/unit_test_fw/inc
>> -std=gnu11 -Wall -fno-strict-aliasing -Werror -fPIC
>> -D_FORTIFY_SOURCE=2 -fstack-protector -
>> DINTERNAL_VERSION_ID='""'
>> -O2 -g -m64 -fmessage-length=0
>> -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables
>> -fasynchronous-unwind-tables -MT clm

Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build failed

2016-08-31 Thread Anders Widell
The commit fixes my build failure, but as Hans has pointed out there is 
still some quality issue with it. I suppose it is Hans' comment Mahesh 
refers to?

regards,

Anders Widell


On 08/31/2016 12:45 PM, Zoran Milinkovic wrote:
> Hi,
>
> I completely missed this email thread and the patch.
> I build OpenSAF with gcc 4.8.4 on Ubuntu, and haven’t had any building 
> problem.
>
> If the patch works for Anders (gcc 5.x and/or 6.x), then I'm ok with the 
> patch. I cannot test the build in my environment.
>
> Mahesh, what kind of comment did you mean ?
>
> Thanks,
> Zoran
>
> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: den 30 augusti 2016 10:4
> To: Hans Nordebäck; Zoran Milinkovic
> Cc: praveen malviya; opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build 
> failed
>
> Hi HansN,
>
> Thanks for the point , Zoran Milinkovic owner this code, jut to unblock the 
> build issue,  as workaround I pushed it , I will fw this comment to him.
>
> Zoran Milinkovic,
>
> Can you please incorporate the comment.
>
> -AVM
>
> On 8/30/2016 1:08 PM, Hans Nordebäck wrote:
>> Hi Mahesh,
>>
>> One question/comment below. /Thanks HansN
>>
>> -Original Message-
>> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
>> Sent: den 30 augusti 2016 08:09
>> To: praveen malviya ;
>> opensaf-devel@lists.sourceforge.net
>> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3
>> build failed
>>
>> Ok,
>>
>> I just pushed
>>
>> changeset: 7989:4d6caf6903cd
>> tag: tip
>> user:A V Mahesh 
>> date:Tue Aug 30 11:05:31 2016 +0530
>> summary: clm: test code build fix [#1906]
>>
>> -AVM
>>
>> On 8/30/2016 11:28 AM, praveen malviya wrote:
>>> Hi Mahesh,
>>>
>>> I think this patch can be pushed as a workaround patch. Any
>>> improvement on it can be taken up by CLM maintainers post FC tag.
>>>
>>> Thanks,
>>> Praveen
>>>
>>> On 30-Aug-16 11:17 AM, A V Mahesh wrote:
 Hi Zoran Milinkovic,

 If  below changes ok , we can push directly.

 
 =
 ==

 diff --git a/tests/clmsv/tet_ClmLongRdn.c
 b/tests/clmsv/tet_ClmLongRdn.c
 --- a/tests/clmsv/tet_ClmLongRdn.c
 +++ b/tests/clmsv/tet_ClmLongRdn.c
 @@ -188,19 +188,21 @@ static SaClmCallbacksT_4 clmCallback4 =
 static SaClmCallbacksT clmCallback = { nodeGetCallBack,
 clmTrackCallback };

 static void unlock_node(char *nodename) {
 +   int rc;
char command[1024];

// Unlock the node
sprintf(command, "immadm -o 1 %s", nodename);
 -   system(command);
 +   assert(rc = system(command) != -1);
>> [HansN]
>> I know this is test code but side effects in asserts is to be avoided, but, 
>> shouldn't the code be like:
>> rc = system(command);
>> int status = WEXITSTATUS(rc);
>> osaf_assert(status != SA_AIS_OK);  ?
>>
 +
 }

 static void lock_node(char *nodename) {
 +   int rc;
char command[1024];
 -
// Lock the node
sprintf(command, "immadm -o 2 %s", nodename);
 -   system(command);
 +   assert(rc = system(command) != -1);
 }

 static void remove_node(char *nodename) { @@ -209,7 +211,7 @@
 static void remove_node(char *nodename)

// Lock the node
sprintf(command, "immadm -o 2 %s", nodename);
 -   system(command);
 +   assert(rc = system(command) != -1);

// Remove the node
sprintf(command, "immcfg -d %s", nodename);
 
 =
 ==


 -AVM

 On 8/30/2016 10:46 AM, A V Mahesh wrote:
> Hi All,
>
> I just build  http://hg.code.sf.net/p/opensaf/staging with
> following changeset as sanity test
>
> it build failed with .
>
> changeset:   7982:106230d848a6
> tag: tip
> parent:  7979:aec46cc64cc8
> user:Anders Widell 
> date:Mon Aug 29 19:29:55 2016 +0200
> summary: uml: Update the UML environment [#1979]
>
> ===
> =
> ===
>
>
> /gcc -DHAVE_CONFIG_H -I. -I../.. -DSA_CLM_B01=1 -I../..
> -I../../osaf/libs/saf/include -I../../osaf/libs/core/include
> -I../../osaf/libs/core/leap/include
> -I../../osaf/libs/core/mds/include
> -I../../osaf/libs/core/common/include
> -I../../osaf/libs/core/cplusplus -I../../tests/unit_test_fw/inc
> -std=gnu11 -Wall -fno-strict-aliasing -Werror -fPIC
> -D_FORTIFY_SOURCE=2 -fstack-protector -DINTERNAL_VERSION_ID='""'
> -O2 -g -m64 -fmessage-length=0
> -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables
> 

Re: [devel] [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]

2016-08-31 Thread Hans Nordebäck
Hi Mahesh,
Any updates on this?

/Regards HansN

-Original Message-
From: Anders Widell 
Sent: den 25 augusti 2016 13:11
To: A V Mahesh ; Hans Nordebäck 
; mathi.naic...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1 of 1] MDS: Log TIPC dropped messages [#1957]

Hi!

This is what the TIPC user documentation says about TIPC_DEST_DROPPABLE: 
"This option governs the handling of messages sent by the socket if the message 
cannot be delivered to its destination, either because the receiver is 
congested or because the specified receiver does not exist. 
If enabled, the message is discarded; otherwise the message is returned to the 
sender."

This is what the TIPC user documentation says about the return value from the 
recvmsg() system call: "When used with a connectionless socket, a return value 
of 0 indicates the arrival of a returned data message that was originally sent 
by this socket."

I think the documentation is pretty clear. If you set TIPC_DEST_DROPPABLE to 
true, the receiver can discard messages e.g. when the receive buffer is full. 
The sender will not be notified in this case. If TIPC_DEST_DROPPABLE is set to 
false, the message will be returned to the sender in case of a full receive 
buffer. The sender knows that it has received such a returned message when the 
recvmsg() call returns zero.

regards,
Anders Widell

On 08/25/2016 11:30 AM, A V Mahesh wrote:
> Hi HansN,
>
>
> On 8/23/2016 5:22 PM, Hans Nordebäck wrote:
>
>> Hi Mahesh,
>>
>> Yes, this is my understanding too, if TIPC_DROPPABLE = true tipc may 
>> drop messages silently,  at receive sock buffer full condition,  but 
>> do not return any ancillary message.
>> If TIPC_DROPPABLE = false tipc may drop message but will send an 
>> ancillary message to inform about TIPC_ERR_OVERLOAD.
> [AVM]
>
> My observation are understanding is different, based on TIPC code and 
> Linux TIPC 2.0 Programmer's Guide , that the TIPC_ERR_OVERLOAD error 
> returned when TIPC is unable to enqueue an incoming message on the 
> receiving socket's receive queue irrelevant of TIPC_DEST_DROPPABLE 
> enabled or disabled.
>
> The only difference between TIPC_DEST_DROPPABLE enabled or disabled is 
> , If  TIPC_DEST_DROPPABLE enabled, the message is discarded and
> recvmsg() returned size is ZERO and application will get errors, if 
> TIPC_DEST_DROPPABLE disabled  the message is returned to the sender it 
> means the recvmsg() returned size is user send data size and 
> application will get errors .
>
> I did check the TIPC code and documentations  and I haven't  get any 
> evidences that  TIPC_ERR_OVERLOAD error code will be send only If 
> TIPC_DEST_DROPPABLE = false.
>
> Even while testing #1227
> (https://sourceforge.net/p/opensaf/mailman/message/33207717/) my 
> observations and understanding was, an individual TIPC socket is only 
> allowed to queue up
> OVERLOAD_LIMIT_BASE/2 messages of the lowest importance level before 
> it starts rejecting them.
> Once a socket receiving queue length exceeds the maximum limit value, 
> the receiving socket will send out a reject message  with 
> TIPC_ERR_OVERLOAD error code with cmsg_type as 
> TIPC_ERRINFO/TIPC_RETDATA, and the tipc code and Linux TIPC 2.0 
> Programmer's Guide  confirmed the same .
>
> tipc/socket.c
> ===
> /* Reject message if there isn't room to queue it */
>
> recv_q_len = (u32)atomic_read(&tipc_queue_size);
> if (unlikely(recv_q_len >= OVERLOAD_LIMIT_BASE)) {
> if (rx_queue_full(msg, recv_q_len, OVERLOAD_LIMIT_BASE))
> return TIPC_ERR_OVERLOAD;
> }
> recv_q_len = skb_queue_len(&sk->sk_receive_queue);
> if (unlikely(recv_q_len >= (OVERLOAD_LIMIT_BASE / 2))) {
> if (rx_queue_full(msg, recv_q_len, OVERLOAD_LIMIT_BASE / 2))
> return TIPC_ERR_OVERLOAD;
> }
> ===
>
>
> 2.1.17. setsockopt() of  TIPC 2.0 Programmer's Guide 
> ===
> TIPC_DEST_DROPPABLE
> This option governs the handling of messages sent by the socket if the 
> message cannot be delivered to its destination, either because the 
> receiver is congested or because the specified receiver does not 
> exist. If enabled, the message is discarded; otherwise the message is 
> returned to the sender.
>
> By default, this option is disabled for SOCK_SEQPACKET and SOCK_STREAM 
> socket types, and enabled for SOCK_RDM and SOCK_DGRAM, This 
> arrangement ensures proper teardown of failed connections when 
> connection-oriented data transfer is used, without increasing the 
> complexity of connectionless data transfer.
>
> TIPC_SRC_DROPPABLE
> This option governs the handling of messages sent by the socket if 
> link congestion occurs. If enabled, the message is discarded; 
> otherwise the system queues the message for later transmission.
> By default, this option is disabled for SOCK_SEQPACKET, SOCK_STREAM, 
> and SOCK_RDM socket types (result

Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build failed

2016-08-31 Thread Zoran Milinkovic
Hi,

I completely missed this email thread and the patch.
I build OpenSAF with gcc 4.8.4 on Ubuntu, and haven’t had any building problem.

If the patch works for Anders (gcc 5.x and/or 6.x), then I'm ok with the patch. 
I cannot test the build in my environment.

Mahesh, what kind of comment did you mean ?

Thanks,
Zoran

-Original Message-
From: A V Mahesh [mailto:mahesh.va...@oracle.com] 
Sent: den 30 augusti 2016 10:40
To: Hans Nordebäck; Zoran Milinkovic
Cc: praveen malviya; opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build failed

Hi HansN,

Thanks for the point , Zoran Milinkovic owner this code, jut to unblock the 
build issue,  as workaround I pushed it , I will fw this comment to him.

Zoran Milinkovic,

Can you please incorporate the comment.

-AVM

On 8/30/2016 1:08 PM, Hans Nordebäck wrote:
> Hi Mahesh,
>
> One question/comment below. /Thanks HansN
>
> -Original Message-
> From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> Sent: den 30 augusti 2016 08:09
> To: praveen malviya ; 
> opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 
> build failed
>
> Ok,
>
> I just pushed
>
> changeset: 7989:4d6caf6903cd
> tag: tip
> user:A V Mahesh 
> date:Tue Aug 30 11:05:31 2016 +0530
> summary: clm: test code build fix [#1906]
>
> -AVM
>
> On 8/30/2016 11:28 AM, praveen malviya wrote:
>> Hi Mahesh,
>>
>> I think this patch can be pushed as a workaround patch. Any 
>> improvement on it can be taken up by CLM maintainers post FC tag.
>>
>> Thanks,
>> Praveen
>>
>> On 30-Aug-16 11:17 AM, A V Mahesh wrote:
>>> Hi Zoran Milinkovic,
>>>
>>> If  below changes ok , we can push directly.
>>>
>>> 
>>> =
>>> ==
>>>
>>> diff --git a/tests/clmsv/tet_ClmLongRdn.c 
>>> b/tests/clmsv/tet_ClmLongRdn.c
>>> --- a/tests/clmsv/tet_ClmLongRdn.c
>>> +++ b/tests/clmsv/tet_ClmLongRdn.c
>>> @@ -188,19 +188,21 @@ static SaClmCallbacksT_4 clmCallback4 =
>>>static SaClmCallbacksT clmCallback = { nodeGetCallBack, 
>>> clmTrackCallback };
>>>
>>>static void unlock_node(char *nodename) {
>>> +   int rc;
>>>   char command[1024];
>>>
>>>   // Unlock the node
>>>   sprintf(command, "immadm -o 1 %s", nodename);
>>> -   system(command);
>>> +   assert(rc = system(command) != -1);
> [HansN]
> I know this is test code but side effects in asserts is to be avoided, but, 
> shouldn't the code be like:
> rc = system(command);
> int status = WEXITSTATUS(rc);
> osaf_assert(status != SA_AIS_OK);  ?
>
>>> +
>>>}
>>>
>>>static void lock_node(char *nodename) {
>>> +   int rc;
>>>   char command[1024];
>>> -
>>>   // Lock the node
>>>   sprintf(command, "immadm -o 2 %s", nodename);
>>> -   system(command);
>>> +   assert(rc = system(command) != -1);
>>>}
>>>
>>>static void remove_node(char *nodename) { @@ -209,7 +211,7 @@ 
>>> static void remove_node(char *nodename)
>>>
>>>   // Lock the node
>>>   sprintf(command, "immadm -o 2 %s", nodename);
>>> -   system(command);
>>> +   assert(rc = system(command) != -1);
>>>
>>>   // Remove the node
>>>   sprintf(command, "immcfg -d %s", nodename); 
>>> 
>>> =
>>> ==
>>>
>>>
>>> -AVM
>>>
>>> On 8/30/2016 10:46 AM, A V Mahesh wrote:
 Hi All,

 I just build  http://hg.code.sf.net/p/opensaf/staging with 
 following changeset as sanity test

 it build failed with .

 changeset:   7982:106230d848a6
 tag: tip
 parent:  7979:aec46cc64cc8
 user:Anders Widell 
 date:Mon Aug 29 19:29:55 2016 +0200
 summary: uml: Update the UML environment [#1979]

 ===
 =
 ===


 /gcc -DHAVE_CONFIG_H -I. -I../.. -DSA_CLM_B01=1 -I../..
 -I../../osaf/libs/saf/include -I../../osaf/libs/core/include 
 -I../../osaf/libs/core/leap/include
 -I../../osaf/libs/core/mds/include
 -I../../osaf/libs/core/common/include
 -I../../osaf/libs/core/cplusplus -I../../tests/unit_test_fw/inc
 -std=gnu11 -Wall -fno-strict-aliasing -Werror -fPIC
 -D_FORTIFY_SOURCE=2 -fstack-protector -DINTERNAL_VERSION_ID='""' 
 -O2 -g -m64 -fmessage-length=0
 -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables 
 -fasynchronous-unwind-tables -MT clmtest-tet_ClmLongRdn.o -MD -MP 
 -MF .deps/clmtest-tet_ClmLongRdn.Tpo -c -o clmtest-tet_ClmLongRdn.o 
 `test -f 'tet_ClmLongRdn.c' || echo './'`tet_ClmLongRdn.c
 tet_ClmLongRdn.c: In function ‘lock_node’:
 tet_ClmLongRdn.c:203:8: error: ignoring return value of 
 ‘system’, declared with attribute warn_unused_result 
 [-Werror=unused-result]
 

Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build failed

2016-08-31 Thread Mathivanan Naickan Palanivelu
Iam looking into this.

Thanks,
Mathi.

> -Original Message-
> From: A V Mahesh
> Sent: Tuesday, August 30, 2016 2:10 PM
> To: Hans Nordebäck; Zoran Milinkovic
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3 build 
> failed
> 
> Hi HansN,
> 
> Thanks for the point , Zoran Milinkovic owner this code, jut to unblock the
> build issue,  as workaround I pushed it , I will fw this comment to him.
> 
> Zoran Milinkovic,
> 
> Can you please incorporate the comment.
> 
> -AVM
> 
> On 8/30/2016 1:08 PM, Hans Nordebäck wrote:
> > Hi Mahesh,
> >
> > One question/comment below. /Thanks HansN
> >
> > -Original Message-
> > From: A V Mahesh [mailto:mahesh.va...@oracle.com]
> > Sent: den 30 augusti 2016 08:09
> > To: praveen malviya ;
> > opensaf-devel@lists.sourceforge.net
> > Subject: Re: [devel] staging changeset: 7982 with gcc version 4.8.3
> > build failed
> >
> > Ok,
> >
> > I just pushed
> >
> > changeset: 7989:4d6caf6903cd
> > tag: tip
> > user:A V Mahesh 
> > date:Tue Aug 30 11:05:31 2016 +0530
> > summary: clm: test code build fix [#1906]
> >
> > -AVM
> >
> > On 8/30/2016 11:28 AM, praveen malviya wrote:
> >> Hi Mahesh,
> >>
> >> I think this patch can be pushed as a workaround patch. Any
> >> improvement on it can be taken up by CLM maintainers post FC tag.
> >>
> >> Thanks,
> >> Praveen
> >>
> >> On 30-Aug-16 11:17 AM, A V Mahesh wrote:
> >>> Hi Zoran Milinkovic,
> >>>
> >>> If  below changes ok , we can push directly.
> >>>
> >>>
> ==
> ==
> >>> =
> >>> ==
> >>>
> >>> diff --git a/tests/clmsv/tet_ClmLongRdn.c
> >>> b/tests/clmsv/tet_ClmLongRdn.c
> >>> --- a/tests/clmsv/tet_ClmLongRdn.c
> >>> +++ b/tests/clmsv/tet_ClmLongRdn.c
> >>> @@ -188,19 +188,21 @@ static SaClmCallbacksT_4 clmCallback4 =
> >>>static SaClmCallbacksT clmCallback = { nodeGetCallBack,
> >>> clmTrackCallback };
> >>>
> >>>static void unlock_node(char *nodename) {
> >>> +   int rc;
> >>>   char command[1024];
> >>>
> >>>   // Unlock the node
> >>>   sprintf(command, "immadm -o 1 %s", nodename);
> >>> -   system(command);
> >>> +   assert(rc = system(command) != -1);
> > [HansN]
> > I know this is test code but side effects in asserts is to be avoided, but,
> shouldn't the code be like:
> > rc = system(command);
> > int status = WEXITSTATUS(rc);
> > osaf_assert(status != SA_AIS_OK);  ?
> >
> >>> +
> >>>}
> >>>
> >>>static void lock_node(char *nodename) {
> >>> +   int rc;
> >>>   char command[1024];
> >>> -
> >>>   // Lock the node
> >>>   sprintf(command, "immadm -o 2 %s", nodename);
> >>> -   system(command);
> >>> +   assert(rc = system(command) != -1);
> >>>}
> >>>
> >>>static void remove_node(char *nodename) { @@ -209,7 +211,7 @@
> >>> static void remove_node(char *nodename)
> >>>
> >>>   // Lock the node
> >>>   sprintf(command, "immadm -o 2 %s", nodename);
> >>> -   system(command);
> >>> +   assert(rc = system(command) != -1);
> >>>
> >>>   // Remove the node
> >>>   sprintf(command, "immcfg -d %s", nodename);
> >>>
> ==
> ==
> >>> =
> >>> ==
> >>>
> >>>
> >>> -AVM
> >>>
> >>> On 8/30/2016 10:46 AM, A V Mahesh wrote:
>  Hi All,
> 
>  I just build  http://hg.code.sf.net/p/opensaf/staging with
>  following changeset as sanity test
> 
>  it build failed with .
> 
>  changeset:   7982:106230d848a6
>  tag: tip
>  parent:  7979:aec46cc64cc8
>  user:Anders Widell 
>  date:Mon Aug 29 19:29:55 2016 +0200
>  summary: uml: Update the UML environment [#1979]
> 
> 
> ==
> =
>  =
>  ===
> 
> 
>  /gcc -DHAVE_CONFIG_H -I. -I../.. -DSA_CLM_B01=1 -I../..
>  -I../../osaf/libs/saf/include -I../../osaf/libs/core/include
>  -I../../osaf/libs/core/leap/include
>  -I../../osaf/libs/core/mds/include
>  -I../../osaf/libs/core/common/include
>  -I../../osaf/libs/core/cplusplus -I../../tests/unit_test_fw/inc
>  -std=gnu11 -Wall -fno-strict-aliasing -Werror -fPIC
>  -D_FORTIFY_SOURCE=2 -fstack-protector -
> DINTERNAL_VERSION_ID='""'
>  -O2 -g -m64 -fmessage-length=0
>  -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables
>  -fasynchronous-unwind-tables -MT clmtest-tet_ClmLongRdn.o -MD -
> MP
>  -MF .deps/clmtest-tet_ClmLongRdn.Tpo -c -o clmtest-
> tet_ClmLongRdn.o
>  `test -f 'tet_ClmLongRdn.c' || echo './'`tet_ClmLongRdn.c
>  tet_ClmLongRdn.c: In function ‘lock_node’:
>  tet_ClmLongRdn.c:203:8: error: ignoring return value of
>  ‘system’, declared with attribute warn_unused_result [-
> Werror=unused-result]
>   system(command);
> 

[devel] [PATCH 1 of 1] log: fix cppcheck with performance severity [#1975]

2016-08-31 Thread Canh Van Truong
 osaf/services/saf/logsv/lgs/lgs_config.cc |  35 --
 osaf/services/saf/logsv/lgs/lgs_file.cc   |   4 +-
 2 files changed, 16 insertions(+), 23 deletions(-)


Fix performance reported by cppcheck.

diff --git a/osaf/services/saf/logsv/lgs/lgs_config.cc 
b/osaf/services/saf/logsv/lgs/lgs_config.cc
--- a/osaf/services/saf/logsv/lgs/lgs_config.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_config.cc
@@ -137,32 +137,25 @@ typedef struct _lgs_conf_t {
   lgs_conf_flg_t logDataGroupname_cnfflag;
   lgs_conf_flg_t logStreamFileFormat_cnfflag;
 
-  _lgs_conf_t() {
-/*
- * For the following flags, LGS_CNF_DEF means that no external
- * configuration exists and the corresponding attributes hard-coded
- * default value is used.Is set to false if configuration is found in
- * IMM object or environment variable.
- * See function lgs_logconf_get() for more info.
- */
+  _lgs_conf_t()
+  : logRootDirectory(lgs_conf_def.logRootDirectory),
+logRootDirectory_cnfflag(LGS_CNF_DEF),
+logMaxLogrecsize_cnfflag(LGS_CNF_DEF),
+logStreamSystemHighLimit_cnfflag(LGS_CNF_DEF),
+logStreamSystemLowLimit_cnfflag(LGS_CNF_DEF),
+logStreamAppHighLimit_cnfflag(LGS_CNF_DEF),
+logStreamAppLowLimit_cnfflag(LGS_CNF_DEF),
+logMaxApplicationStreams_cnfflag(LGS_CNF_DEF),
+logFileIoTimeout_cnfflag(LGS_CNF_DEF),
+logFileSysConfig_cnfflag(LGS_CNF_DEF),
+logDataGroupname_cnfflag(LGS_CNF_DEF),
+logStreamFileFormat_cnfflag(LGS_CNF_DEF)
+  {
 OpenSafLogConfig_object_exist = false;
-logRootDirectory_cnfflag = LGS_CNF_DEF;
-logStreamSystemHighLimit_cnfflag = LGS_CNF_DEF;
-logStreamSystemLowLimit_cnfflag = LGS_CNF_DEF;
-logStreamAppHighLimit_cnfflag = LGS_CNF_DEF;
-logStreamAppLowLimit_cnfflag = LGS_CNF_DEF;
-logDataGroupname_cnfflag = LGS_CNF_DEF;
 /*
  * The following attributes cannot be configured in the config file
  * Will be set to false if the attribute exists in the IMM config object
  */
-logMaxLogrecsize_cnfflag = LGS_CNF_DEF;
-logMaxApplicationStreams_cnfflag = LGS_CNF_DEF;
-logFileIoTimeout_cnfflag = LGS_CNF_DEF;
-logFileSysConfig_cnfflag = LGS_CNF_DEF;
-logStreamFileFormat_cnfflag = LGS_CNF_DEF;
-
-logRootDirectory = lgs_conf_def.logRootDirectory;
 (void) strcpy(logDataGroupname, lgs_conf_def.logDataGroupname);
 (void) strcpy(logStreamFileFormat, lgs_conf_def.logStreamFileFormat);
 logMaxLogrecsize = lgs_conf_def.logMaxLogrecsize;
diff --git a/osaf/services/saf/logsv/lgs/lgs_file.cc 
b/osaf/services/saf/logsv/lgs/lgs_file.cc
--- a/osaf/services/saf/logsv/lgs/lgs_file.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_file.cc
@@ -51,11 +51,11 @@ struct file_communicate {
   size_t outdata_size;
   void *outdata_ptr;  /* Out data from handlers */
 
-  file_communicate() {
+  file_communicate() : request_code(LGSF_NOREQ)
+  {
 answer_f = false;
 request_f = false;
 timeout_f = false;
-request_code = LGSF_NOREQ;
 return_code = LGSF_NORETC;
 indata_ptr = NULL;
 outdata_ptr = NULL;

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


[devel] [PATCH 0 of 1] Review Request for log: fix cppcheck with performance severity [#1975]

2016-08-31 Thread Canh Van Truong
Summary: log: fix cppcheck with performance severity [#1975]
Review request for Trac Ticket(s): #1975
Peer Reviewer(s): Vu, Lennart, Mahesh
Pull request to: Vu
Affected branch(es): default
Development branch: default


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
 <>

changeset b0e067eac8805b29ff6ae0c7be278dc6b4b29da3
Author: Canh Van Truong 
Date:   Wed, 31 Aug 2016 14:02:16 +0700

log: fix cppcheck with performance severity [#1975]

Fix performance reported by cppcheck.


Complete diffstat:
--
 osaf/services/saf/logsv/lgs/lgs_config.cc |  35 
++-
 osaf/services/saf/logsv/lgs/lgs_file.cc   |   4 ++--
 2 files changed, 16 insertions(+), 23 deletions(-)


Testing Commands:
-
 <>


Testing, Expected Results:
--
 <>


Conditions of Submission:
-
Ack from reviewers


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


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


Re: [devel] [PATCH 0 of 1] Review Request for imm: Add AdmReqContinuation only when fevs message is successfully forwarded [#1977]

2016-08-31 Thread Zoran Milinkovic
Hi Hung,

Reviewed the patch.
Ack.

Thanks,
Zoran

-Original Message-
From: Hung Nguyen [mailto:hung.d.ngu...@dektech.com.au] 
Sent: den 29 augusti 2016 12:47
To: Zoran Milinkovic; reddy.neelaka...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: [PATCH 0 of 1] Review Request for imm: Add AdmReqContinuation only 
when fevs message is successfully forwarded [#1977]

Summary: imm: Add AdmReqContinuation only when fevs message is successfully 
forwarded [#1977] Review request for Trac Ticket(s): 1977 Peer Reviewer(s): 
Zoran, Neel Pull request to:
Affected branch(es): 4.7, 5.0, 5.1
Development branch: 5.1


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-


changeset 3b7ce79c8548dab1eba1336c1993f802aa14a49b
Author: Hung Nguyen 
Date:   Mon, 29 Aug 2016 17:44:19 +0700

imm: Add AdmReqContinuation only when fevs message is successfully 
forwarded
[#1977]

Add AdmReqContinuation only when fevs message is successfully forwarded.


Complete diffstat:
--
 osaf/services/saf/immsv/immnd/immnd_evt.c |  112 
+++-
 1 files changed, 55 insertions(+), 57 deletions(-)


Testing Commands:
-



Testing, Expected Results:
--



Conditions of Submission:
-
Ack from reviewers.


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  n  n
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.


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


Re: [devel] [PATCH 1 of 1] amfnd: Incorrect ER messages in syslog [#1989]

2016-08-31 Thread Hans Nordebäck
Ack, code review only/Thanks HansN

-Original Message-
From: Long HB Nguyen [mailto:long.hb.ngu...@dektech.com.au] 
Sent: den 31 augusti 2016 06:29
To: Gary Lee ; Hans Nordebäck 
; Minh Hon Chau ; 
nagendr...@oracle.com; praveen.malv...@oracle.com; Long Hong Buu Nguyen 

Cc: opensaf-devel@lists.sourceforge.net
Subject: [PATCH 1 of 1] amfnd: Incorrect ER messages in syslog [#1989]

 osaf/services/saf/amf/amfnd/su.cc |  4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/osaf/services/saf/amf/amfnd/su.cc 
b/osaf/services/saf/amf/amfnd/su.cc
--- a/osaf/services/saf/amf/amfnd/su.cc
+++ b/osaf/services/saf/amf/amfnd/su.cc
@@ -417,11 +417,11 @@ uint32_t avnd_evt_avd_info_su_si_assign_
} else {
if (osaf_extended_name_length(&info->si_name) > 0) {
if (avnd_su_si_rec_get(cb, info_su_name, 
Amf::to_string(&info->si_name)) == nullptr)
-   LOG_WA("susi_assign_evh: '%s' is not assigned 
to '%s'",
+   LOG_ER("susi_assign_evh: '%s' is not assigned 
to '%s'",

osaf_extended_name_borrow(&info->si_name), su->name.c_str());
} else {
if (m_NCS_DBLIST_FIND_FIRST(&su->si_list) == nullptr) {
-   LOG_ER("susi_assign_evh: '%s' has no 
assignments", su->name.c_str());
+   LOG_WA("susi_assign_evh: '%s' has no 
assignments", su->name.c_str());
/* Some times AMFD sends redundant message for 
removal of assignments.
   If removal of assignments is already done 
for the SU then complete
   the assignment process here.

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