Re: [devel] [PATCH 1 of 1] log: fix cppcheck with performance severity [#1975]
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]
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]
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]
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
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]
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]
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]
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]
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
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
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
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
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]
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
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
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]
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]
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]
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]
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