Re: [devel] [PATCH 2 of 3] amfnd: Fix all Cppcheck 1.77 issues [#2341] V2

2017-03-14 Thread A V Mahesh
Hi Gary,

Sorry I over looked it ,  will  see and fix it , while fixing other  Amf 
Dev review comments ( may be in V3 ).

-AVM

On 3/15/2017 11:01 AM, Gary Lee wrote:
> Hi Mahesh
>
> Did you see my proposed change?
>
> The ‘delete rec’ statement below will cause a double free, unless I’m 
> mistaken.
>
> Thanks
> Gary
>
>   void avnd_comp_pm_rec_del(AVND_CB *cb, AVND_COMP *comp, 
> AVND_COMP_PM_REC *rec)
>   {
>  -uint32_t rc = NCSCC_RC_SUCCESS;
>   SaUint64T pid = rec->pid;
>   TRACE_ENTER2("Comp '%s'", comp->name.c_str());
>   
>   /* delete the PM_REC from pm_list */
>  -rc = ncs_db_link_list_del(>pm_list, (uint8_t *)>pid);
>  +uint32_t rc = ncs_db_link_list_del(>pm_list, (uint8_t 
> *)>pid);
>   if (NCSCC_RC_SUCCESS != rc) {
>   LOG_NO("PM Rec doesn't exist in Comp '%s' of pid %llu", 
> comp->name.c_str(), pid);
>   }
>  -rec = nullptr;  /* rec is no more, dont use it */
>  +
>  +delete rec; /* rec is no more, dont use it */
>
>
>
> -Original Message-
> From: A V Mahesh 
> Date: Wednesday, 15 March 2017 at 3:30 pm
> To: , gary , 
> , 
> Cc: 
> Subject: [PATCH 2 of 3] amfnd: Fix all Cppcheck 1.77 issues [#2341] V2
>
>   src/amf/amfnd/amfnd.cc   |  15 +--
>   src/amf/amfnd/cbq.cc |   9 +++--
>   src/amf/amfnd/chc.cc |  29 +++--
>   src/amf/amfnd/clc.cc |  37 ++---
>   src/amf/amfnd/comp.cc|   8 +++-
>   src/amf/amfnd/compdb.cc  |  17 -
>   src/amf/amfnd/cpm.cc |  13 +
>   src/amf/amfnd/di.cc  |  35 +--
>   src/amf/amfnd/err.cc |  35 ++-
>   src/amf/amfnd/evt.cc |   3 +--
>   src/amf/amfnd/hcdb.cc|  13 +++--
>   src/amf/amfnd/imm.cc |   5 ++---
>   src/amf/amfnd/main.cc|  13 +
>   src/amf/amfnd/mds.cc |  20 +++-
>   src/amf/amfnd/pg.cc  |  33 +++--
>   src/amf/amfnd/proxy.cc   |   8 +++-
>   src/amf/amfnd/proxydb.cc |  17 +
>   src/amf/amfnd/sidb.cc|   3 +--
>   src/amf/amfnd/su.cc  |   3 +--
>   src/amf/amfnd/sudb.cc|   5 ++---
>   src/amf/amfnd/susm.cc|  19 +--
>   src/amf/amfnd/util.cc|   2 +-
>   22 files changed, 143 insertions(+), 199 deletions(-)
>  
>  
>  V2 Re-based changes on OpenSAF 5.2.RC1 tagged code.
>  [src/amf/amfnd/amfnd.cc:58] -> [src/amf/amfnd/amfnd.cc:63]: (style) 
> Variable 'del_cbk' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/amfnd.cc:59] -> [src/amf/amfnd/amfnd.cc:65]: (style) 
> Variable 'o_comp' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/amfnd.cc:191] -> [src/amf/amfnd/amfnd.cc:199]: (style) 
> Variable 'o_comp' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/amfnd.cc:249] -> [src/amf/amfnd/amfnd.cc:250]: (style) 
> Variable 'res' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/cbq.cc:441] -> [src/amf/amfnd/cbq.cc:442]: (style) 
> Variable 'temp_csi' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/cbq.cc:734] -> [src/amf/amfnd/cbq.cc:748]: (style) 
> Variable 'rc' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/cbq.cc:1132] -> [src/amf/amfnd/cbq.cc:1133]: (style) 
> Variable 'temp_csi' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/chc.cc:842]: (style) The scope of the variable 'cbk_rec' 
> can be reduced.
>  [src/amf/amfnd/clc.cc:1402] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
> inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
> redundant or there is possible null pointer dereference: curr_rec.
>  [src/amf/amfnd/clc.cc:1403] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
> inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
> redundant or there is possible null pointer dereference: curr_rec.
>  [src/amf/amfnd/clc.cc:1404] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
> inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
> redundant or there is possible null pointer dereference: curr_rec.
>  [src/amf/amfnd/clc.cc:610] -> [src/amf/amfnd/clc.cc:616]: (style) 
> Variable 'clc_evt' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/clc.cc:1708] -> [src/amf/amfnd/clc.cc:1712]: (style) 
> Variable 'rc' is reassigned a value before the old one has been used.
>  [src/amf/amfnd/clc.cc:1749] -> 

Re: [devel] [PATCH 2 of 3] amfnd: Fix all Cppcheck 1.77 issues [#2341] V2

2017-03-14 Thread Gary Lee
Hi Mahesh

Did you see my proposed change?

The ‘delete rec’ statement below will cause a double free, unless I’m mistaken.

Thanks
Gary

 void avnd_comp_pm_rec_del(AVND_CB *cb, AVND_COMP *comp, AVND_COMP_PM_REC 
*rec)
 {
-   uint32_t rc = NCSCC_RC_SUCCESS;
SaUint64T pid = rec->pid;
TRACE_ENTER2("Comp '%s'", comp->name.c_str());
 
/* delete the PM_REC from pm_list */
-   rc = ncs_db_link_list_del(>pm_list, (uint8_t *)>pid);
+   uint32_t rc = ncs_db_link_list_del(>pm_list, (uint8_t 
*)>pid);
if (NCSCC_RC_SUCCESS != rc) {
LOG_NO("PM Rec doesn't exist in Comp '%s' of pid %llu", 
comp->name.c_str(), pid);
}
-   rec = nullptr;  /* rec is no more, dont use it */
+
+   delete rec; /* rec is no more, dont use it */



-Original Message-
From: A V Mahesh 
Date: Wednesday, 15 March 2017 at 3:30 pm
To: , gary , 
, 
Cc: 
Subject: [PATCH 2 of 3] amfnd: Fix all Cppcheck 1.77 issues [#2341] V2

 src/amf/amfnd/amfnd.cc   |  15 +--
 src/amf/amfnd/cbq.cc |   9 +++--
 src/amf/amfnd/chc.cc |  29 +++--
 src/amf/amfnd/clc.cc |  37 ++---
 src/amf/amfnd/comp.cc|   8 +++-
 src/amf/amfnd/compdb.cc  |  17 -
 src/amf/amfnd/cpm.cc |  13 +
 src/amf/amfnd/di.cc  |  35 +--
 src/amf/amfnd/err.cc |  35 ++-
 src/amf/amfnd/evt.cc |   3 +--
 src/amf/amfnd/hcdb.cc|  13 +++--
 src/amf/amfnd/imm.cc |   5 ++---
 src/amf/amfnd/main.cc|  13 +
 src/amf/amfnd/mds.cc |  20 +++-
 src/amf/amfnd/pg.cc  |  33 +++--
 src/amf/amfnd/proxy.cc   |   8 +++-
 src/amf/amfnd/proxydb.cc |  17 +
 src/amf/amfnd/sidb.cc|   3 +--
 src/amf/amfnd/su.cc  |   3 +--
 src/amf/amfnd/sudb.cc|   5 ++---
 src/amf/amfnd/susm.cc|  19 +--
 src/amf/amfnd/util.cc|   2 +-
 22 files changed, 143 insertions(+), 199 deletions(-)


V2 Re-based changes on OpenSAF 5.2.RC1 tagged code.
[src/amf/amfnd/amfnd.cc:58] -> [src/amf/amfnd/amfnd.cc:63]: (style) 
Variable 'del_cbk' is reassigned a value before the old one has been used.
[src/amf/amfnd/amfnd.cc:59] -> [src/amf/amfnd/amfnd.cc:65]: (style) 
Variable 'o_comp' is reassigned a value before the old one has been used.
[src/amf/amfnd/amfnd.cc:191] -> [src/amf/amfnd/amfnd.cc:199]: (style) 
Variable 'o_comp' is reassigned a value before the old one has been used.
[src/amf/amfnd/amfnd.cc:249] -> [src/amf/amfnd/amfnd.cc:250]: (style) 
Variable 'res' is reassigned a value before the old one has been used.
[src/amf/amfnd/cbq.cc:441] -> [src/amf/amfnd/cbq.cc:442]: (style) Variable 
'temp_csi' is reassigned a value before the old one has been used.
[src/amf/amfnd/cbq.cc:734] -> [src/amf/amfnd/cbq.cc:748]: (style) Variable 
'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/cbq.cc:1132] -> [src/amf/amfnd/cbq.cc:1133]: (style) 
Variable 'temp_csi' is reassigned a value before the old one has been used.
[src/amf/amfnd/chc.cc:842]: (style) The scope of the variable 'cbk_rec' can 
be reduced.
[src/amf/amfnd/clc.cc:1402] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
redundant or there is possible null pointer dereference: curr_rec.
[src/amf/amfnd/clc.cc:1403] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
redundant or there is possible null pointer dereference: curr_rec.
[src/amf/amfnd/clc.cc:1404] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
redundant or there is possible null pointer dereference: curr_rec.
[src/amf/amfnd/clc.cc:610] -> [src/amf/amfnd/clc.cc:616]: (style) Variable 
'clc_evt' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:1708] -> [src/amf/amfnd/clc.cc:1712]: (style) 
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:1749] -> [src/amf/amfnd/clc.cc:1753]: (style) 
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:1951] -> [src/amf/amfnd/clc.cc:1955]: (style) 
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:2370] -> [src/amf/amfnd/clc.cc:2376]: (style) 
Variable 'csi' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:3052] -> [src/amf/amfnd/clc.cc:3053]: (style) 

Re: [devel] Review Request for amf: Update PR/README for SC absence feature [#2179]

2017-03-14 Thread praveen malviya

Hi Minh,

Ack with two comments in attached read me.


Thanks,
Praveen

On 10-Mar-17 11:14 AM, minh chau wrote:

Hi all,

Please help to review documentation changes. Files are attached, they
also can be found at below links:
https://sourceforge.net/p/opensaf/tickets/_discuss/thread/342e9c61/8b9a/attachment/OpenSAF_AMF_PR_2179.odt

https://sourceforge.net/p/opensaf/tickets/_discuss/thread/342e9c61/a94c/attachment/2179_README.diff


Thanks,
Minh
diff --git a/src/amf/README_SC_ABSENCE b/src/amf/README_SC_ABSENCE
--- a/src/amf/README_SC_ABSENCE
+++ b/src/amf/README_SC_ABSENCE
@@ -44,9 +44,12 @@ amfnd will not reboot the node and enter
 scAbsenceAllowed is configured)
 
 * Escalation and Recovery during SC absence period:
-Restarts will work as normal, but failover or switchover will result in Node
-Failfast. The repair action will be initiated when a SC returns if 
-saAmfSGAutoRepair is enabled.
+Component and su restarts will work as normal. Any fail-over or switch-over at
+component, su, and node level will only cleanup faulty components. Recovery 
will
+be delayed until a SC returns: the fail-over or switch-over of SI assignments
+will be initiated if saAmfSGAutoRepair is enabled, the node will be reboot if 
+saAmfNodeAutoRepair, aAmfNodeFailfastOnTerminationFailure, or 
+saAmfNodeFailfastOnInstantiationFailure is enabled.
[Praveen] I think there is no dependecy of failover and switchover of 
assignents on saAmfSgAutoRepair.
Should the sentence be like this?
 " Recovery (failover or switchvoer of assignments) will be delayed until a SC 
returns. 
When first SC comes up after SC absebce state AMF will perform pending repairs:
-for sufailover recovery if saAmfSGAutoRepair is enabled.
-for node-switchvoer and node failover recoveries if saAmfNodeAutoRepair is 
enabled.
-for INST_FAILED and TERM_FAILED state if saAmfSGAutoRepair and 
saAmfNodeAutoRepair are enabled along with
respective node level attributes saAmfNodeFailfastOnInstantiationFailure or 
saAmfNodeFailfastOnTerminationFailure.
"
.
-for comp-failover recovery, amfnd will re-instantiate comp after assignments 
are switchovered.
"
 * Amfnd detects return of SCs:
 NCSMDS_UP is the event that amfnd uses to detect the presence of an active 
amfd.
@@ -76,16 +79,19 @@ absence recovery. The new attributes are
 
 Only 2N SG is currently supported for admin operation continuation.
 
+* Node reboot during SC absence period:
+The event of node reboot initiated by user during SC absence period 
+may lead to a loss of SI assignments. When a SC returns, AMF Director
+will detect improper SI assignments and recover HA states of assignments. 
+
 LIMITATIONS
 ---
 
-* While both SCs are absent, any failover or switchover escalation will result 
-in node failfast. The events of node reboot, node power off, and node failfast
-will lead to a loss of SI assignments, which are not restored during the SC 
-absence period. The SI assignments may remain in improper states until a SC 
-comes back. Recovery of any lost SI assignments during SC absence period is 
-currently not supported.
-
+* Possible loss of RTA updates and SI assignment messages
+If both SCs go down abruptly (SCs are immediately powered-off for instance),
+AMFD could fail to update RTA to IMM, the SI assignment messages sent from
+AMFND could not reach to AMFD, recovery could be impossible. 
+  
[Praveen] Should be mention the case of loss of assignment reseponse from AMFND 
to AMFD?
Also I think we should mention impact of this loss, something like:
"In case of loss of RTA and SI assignments, AMF will not be able to fully 
recover assignments. Thus application
may go in inconsistent state."
 * SI dependency tolerance timer 
 After a SC comes back, if an unassigned sponsor SI is detected, all its 
 dependent SI(s) assignments are removed regardless of tolerance duration. The 
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues [#2341] V1

2017-03-14 Thread A V Mahesh
Hi Gary,

The V1 patch is failing to apply cleanly on  OpenSAF 5.2.RC1 tagged code, so i 
re-based with V2 ,
also fixed additional issue , that you said in V1 and they are considerable
so I republished V2 with completely.

-AVM


On 3/14/2017 12:39 PM, Gary Lee wrote:
> Hi Mahesh
>
> Perhaps it's easier if you pushed V1 first. Otherwise the patches get even 
> bigger and harder to review. I was referring to regression tests failing 
> without the changes I proposed, when I said legacy tests failed.
>
> thanks
>
>> On 14 Mar 2017, at 6:01 pm, A V Mahesh  wrote:
>>
>> Hi Gary,
>>
>> Previously you found some old application issue and you resolved it is that 
>> related to this path or different issue ?
>>
>> -AVM
>>
>>
>>> On 3/14/2017 12:20 PM, A V Mahesh wrote:
>>> Hi Gar,
>>>
>>> Thanks for the review.
>>>
 On 3/14/2017 11:47 AM, Gary Lee wrote:
 By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, 
 but this is a great improvement.
>>> Ok  will re-run the  Cppcheck and if we find considerable , I will
>>> re-publish the V2 patch.
>>>
>>> -AVM
>>>
 On 3/14/2017 11:47 AM, Gary Lee wrote:
 Hi Mahesh

 Ack for the series (regression tests run) with the following changes.

 By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, 
 but this is a great improvement.

 Thanks
 Gary

 diff --git a/src/amf/amfd/csi.cc b/src/amf/amfd/csi.cc
 --- a/src/amf/amfd/csi.cc
 +++ b/src/amf/amfd/csi.cc
 @@ -1399,12 +1399,13 @@ void avd_csi_remove_csiattr(AVD_CSI *csi
   void avd_csi_add_csiattr(AVD_CSI *csi, AVD_CSI_ATTR *csiattr)
{
 -int cnt = 0;
 +int cnt = 1;
AVD_CSI_ATTR *ptr;
   TRACE_ENTER();
/* Count number of attributes (multivalue) */
ptr = csiattr;
 +osafassert(ptr != nullptr);
while (ptr->attr_next != nullptr) {
cnt++;
ptr = ptr->attr_next;
 diff --git a/src/amf/amfnd/cpm.cc b/src/amf/amfnd/cpm.cc
 --- a/src/amf/amfnd/cpm.cc
 +++ b/src/amf/amfnd/cpm.cc
 @@ -145,7 +145,7 @@ void avnd_comp_pm_rec_del(AVND_CB *cb, A
LOG_NO("PM Rec doesn't exist in Comp '%s' of pid %llu", 
 comp->name.c_str(), pid);
}
-delete rec;/* rec is no more, dont use it */
 +rec = nullptr;/* rec is no more, dont use it */
   /* remove the corresponding element from mon_req list */
rc = avnd_mon_req_del(cb, pid);


 -Original Message-
 From: A V Mahesh 
 Date: Wednesday, 8 March 2017 at 11:28 pm
 To: , gary , 
 , 
 Cc: 
 Subject: [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 
 issues [#2341] V1

   Summary:amf: Fix all Cppcheck 1.77 issues [#2341] V1
   Review request for Trac Ticket(s): #2341
   Peer Reviewer(s): Amf Dev
   Pull request to: <>
   Affected branch(es): default, 5.2
   Development branch: default
 
   Impacted area   Impact y/n
   
Docsn
Build systemn
RPM/packaging   n
Configuration files n
Startup scripts n
SAF servicesn
OpenSAF servicesy
Core libraries  n
Samples n
Tests   n
Other   n
   Comments (indicate scope for each "y" above):
   -
 changeset b8e7593cf4eadaa6169fe635ac0df67560ea875a
   Author:A V Mahesh 
   Date:Wed, 08 Mar 2017 17:52:21 +0530
 amfd: Fix all Cppcheck 1.77 issues [#2341] V1
 [staging/src/amf/amfd/app.cc:285]: (style) The scope of 
 the variable 'i' can
   be reduced. [staging/src/amf/amfd/apptype.cc:137]: (style) 
 Condition 'rc!=0'
   is always false [staging/src/amf/amfd/apptype.cc:69]: (style) 
 The scope of
   the variable 'sg_type' can be reduced. 
 [staging/src/amf/amfd/chkop.cc:1297]
   -> [staging/src/amf/amfd/chkop.cc:1302]: (style) Variable 'uba' 
 is
   reassigned a value before the old one has been used.
   [staging/src/amf/amfd/ckpt_dec.cc:374] ->
   [staging/src/amf/amfd/ckpt_dec.cc:382]: (style) Variable 
 'status' is
   reassigned a value before the old one has been used.
   

[devel] [PATCH 3 of 3] amfa: Fix all Cppcheck 1.77 issues [#2341] V2

2017-03-14 Thread A V Mahesh
 src/amf/agent/ava_hdl.cc  |  13 +
 src/amf/agent/ava_mds.cc  |  12 
 src/amf/agent/ava_op.cc   |  11 ---
 src/amf/amfwd/amf_wdog.c  |   2 +-
 src/amf/common/d2nmsg.c   |   7 ++-
 src/amf/common/n2avamsg.c |   1 -
 src/amf/tools/amf_pm.c|   4 +---
 7 files changed, 17 insertions(+), 33 deletions(-)


V2 Re-based changes on OpenSAF 5.2.RC1 tagged code.
[src/amf/agent/ava_hdl.cc:59] -> [src/amf/agent/ava_hdl.cc:67]: (style) 
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/agent/ava_hdl.cc:420]: (style) The scope of the variable 'rec' can be 
reduced.
[src/amf/agent/ava_hdl.cc:592]: (style) The scope of the variable 'i' can be 
reduced.
[src/amf/agent/ava_mds.cc:86] -> [src/amf/agent/ava_mds.cc:90]: (style) 
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/agent/ava_mds.cc:149] -> [src/amf/agent/ava_mds.cc:158]: (style) 
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/agent/ava_mds.cc:1020] -> [src/amf/agent/ava_mds.cc:1028]: (style) 
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/agent/ava_mds.cc:1103] -> [src/amf/agent/ava_mds.cc:1107]: (style) 
Variable 'msg' is reassigned a value before the old one has been used.
[src/amf/agent/ava_op.cc:221] -> [src/amf/agent/ava_op.cc:222]: (style) 
Variable 'osaf_cbk.saAmfContainedComponentInstantiateCallback' is reassigned a 
value before the old one has been used.
[src/amf/agent/ava_op.cc:240] -> [src/amf/agent/ava_op.cc:241]: (style) 
Variable 'osaf_cbk.saAmfContainedComponentInstantiateCallback' is reassigned a 
value before the old one has been used.
[src/amf/agent/ava_op.cc:241] -> [src/amf/agent/ava_op.cc:242]: (style) 
Variable 'osaf_cbk.saAmfContainedComponentInstantiateCallback' is reassigned a 
value before the old one has been used.
[src/amf/agent/ava_op.cc:120]: (style) The scope of the variable 'cb' can be 
reduced.

diff --git a/src/amf/agent/ava_hdl.cc b/src/amf/agent/ava_hdl.cc
--- a/src/amf/agent/ava_hdl.cc
+++ b/src/amf/agent/ava_hdl.cc
@@ -56,7 +56,6 @@ static bool ava_hdl_cbk_ipc_mbx_del(NCSC
 uint32_t ava_hdl_init(AVA_HDL_DB *hdl_db)
 {
NCS_PATRICIA_PARAMS param;
-   uint32_t rc = NCSCC_RC_SUCCESS;
TRACE_ENTER();
 
memset(, 0, sizeof(NCS_PATRICIA_PARAMS));
@@ -64,7 +63,7 @@ uint32_t ava_hdl_init(AVA_HDL_DB *hdl_db
/* init the hdl db tree */
param.key_size = sizeof(uint32_t);
 
-   rc = ncs_patricia_tree_init(_db->hdl_db_anchor, );
+   uint32_t rc = ncs_patricia_tree_init(_db->hdl_db_anchor, );
if (NCSCC_RC_SUCCESS == rc)
hdl_db->num = 0;
else
@@ -417,7 +416,6 @@ uint32_t ava_hdl_cbk_dispatch_one(AVA_CB
 uint32_t ava_hdl_cbk_dispatch_all(AVA_CB **cb, AVA_HDL_REC **hdl_rec)
 {
AVA_PEND_RESP *list_resp = &(*hdl_rec)->pend_resp;
-   AVA_PEND_CBK_REC *rec = 0;
uint32_t hdl = (*hdl_rec)->hdl;
OsafAmfCallbacksT reg_cbk;
uint32_t rc = SA_AIS_OK;
@@ -428,7 +426,7 @@ uint32_t ava_hdl_cbk_dispatch_all(AVA_CB
 
/* pop all the records from the mailbox & process them */
do {
-   rec = (AVA_PEND_CBK_REC 
*)m_NCS_IPC_NON_BLK_RECEIVE(&(*hdl_rec)->callbk_mbx, NULL);
+   AVA_PEND_CBK_REC *rec = (AVA_PEND_CBK_REC 
*)m_NCS_IPC_NON_BLK_RECEIVE(&(*hdl_rec)->callbk_mbx, NULL);
if (!rec)
break;
 
@@ -589,7 +587,6 @@ uint32_t ava_hdl_cbk_dispatch_block(AVA_
 uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CBK_INFO *info, OsafAmfCallbacksT 
*reg_cbk)
 {
uint32_t rc = SA_AIS_OK;
-   uint32_t i;
TRACE_ENTER2("CallbackType = %d",info->type);
 
/* invoke the corresponding callback */
@@ -678,7 +675,7 @@ uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CB
 
if(ava_B4_ver_used(0)) {
SaAmfProtectionGroupNotificationBufferT_4 buf 
={0};
-   for (i = 0 ; i < pg_track->buf.numberOfItems ; 
i++) {
+   for (uint32_t i = 0 ; i < 
pg_track->buf.numberOfItems ; i++) {
if 
(!ava_sanamet_is_valid(_track->buf.notification[i].member.compName)) {
rc = SA_AIS_ERR_NAME_TOO_LONG;
break;
@@ -716,7 +713,7 @@ uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CB
else /* B01 version is used */
{
SaAmfProtectionGroupNotificationBufferT buf;
-   for (i = 0 ; i < pg_track->buf.numberOfItems ; 
i++) {
+   for (uint32_t i = 0 ; i < 
pg_track->buf.numberOfItems ; i++) {
if 
(!ava_sanamet_is_valid(_track->buf.notification[i].member.compName)) {
rc = SA_AIS_ERR_NAME_TOO_LONG;
  

[devel] [PATCH 1 of 3] amfd: Fix all Cppcheck 1.77 issues [#2341] V2

2017-03-14 Thread A V Mahesh
 src/amf/amfd/app.cc|3 +-
 src/amf/amfd/apptype.cc|   20 ++
 src/amf/amfd/chkop.cc  |3 +-
 src/amf/amfd/ckpt_dec.cc   |   15 ++---
 src/amf/amfd/ckpt_edu.cc   |3 +-
 src/amf/amfd/ckpt_enc.cc   |   60 
 src/amf/amfd/clm.cc|   11 +--
 src/amf/amfd/cluster.cc|   16 ++---
 src/amf/amfd/comp.cc   |   11 +--
 src/amf/amfd/compcstype.cc |7 +-
 src/amf/amfd/comptype.cc   |3 +-
 src/amf/amfd/csi.cc|   23 ++-
 src/amf/amfd/csiattr.cc|9 +--
 src/amf/amfd/ctcstype.cc   |   10 +---
 src/amf/amfd/dmsg.cc   |4 +-
 src/amf/amfd/hlttype.cc|3 +-
 src/amf/amfd/imm.cc|   56 +++
 src/amf/amfd/main.cc   |   19 +-
 src/amf/amfd/mds.cc|4 +-
 src/amf/amfd/ndfsm.cc  |   35 +---
 src/amf/amfd/ndproc.cc |   15 +---
 src/amf/amfd/node.cc   |   24 +++-
 src/amf/amfd/nodegroup.cc  |   15 ++--
 src/amf/amfd/ntf.cc|3 +-
 src/amf/amfd/role.cc   |   42 +-
 src/amf/amfd/sg.cc |   23 +--
 src/amf/amfd/sg_2n_fsm.cc  |   10 +-
 src/amf/amfd/sg_nored_fsm.cc   |3 +-
 src/amf/amfd/sg_npm_fsm.cc |   33 ---
 src/amf/amfd/sg_nway_fsm.cc|   39 +
 src/amf/amfd/sg_nwayact_fsm.cc |5 +-
 src/amf/amfd/sgproc.cc |   19 ++
 src/amf/amfd/sgtype.cc |  114 
 src/amf/amfd/si.cc |   24 ++-
 src/amf/amfd/si_dep.cc |   79 ++--
 src/amf/amfd/siass.cc  |   18 ++---
 src/amf/amfd/sirankedsu.cc |   17 ++---
 src/amf/amfd/su.cc |   11 +--
 src/amf/amfd/sutype.cc |   42 ++
 src/amf/amfd/util.cc   |   69 +---
 40 files changed, 351 insertions(+), 569 deletions(-)


V2 Re-based changes on OpenSAF 5.2.RC1 tagged code .
[src/amf/amfd/app.cc:285]: (style) The scope of the variable 'i' can be reduced.
[src/amf/amfd/apptype.cc:137]: (style) Condition 'rc!=0' is always false
[src/amf/amfd/apptype.cc:89] -> [src/amf/amfd/apptype.cc:84]: (warning, 
inconclusive) Either the condition '(attr=attributes[i++])!=nullptr' is 
redundant or there is possible null pointer dereference: attr.
[src/amf/amfd/apptype.cc:129] -> [src/amf/amfd/apptype.cc:124]: (warning, 
inconclusive) Either the condition '(attr=attributes[i++])!=nullptr' is 
redundant or there is possible null pointer dereference: attr.
[src/amf/amfd/apptype.cc:69]: (style) The scope of the variable 'sg_type' can 
be reduced.
[src/amf/amfd/chkop.cc:1297] -> [src/amf/amfd/chkop.cc:1302]: (style) Variable 
'uba' is reassigned a value before the old one has been used.
[src/amf/amfd/ckpt_dec.cc:374] -> [src/amf/amfd/ckpt_dec.cc:382]: (style) 
Variable 'status' is reassigned a value before the old one has been used.
[src/amf/amfd/ckpt_dec.cc:573] -> [src/amf/amfd/ckpt_dec.cc:577]: (style) 
Variable 'status' is reassigned a value before the old one has been used.
[src/amf/amfd/ckpt_dec.cc:1109]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_edu.cc:51] -> [src/amf/amfd/ckpt_edu.cc:56]: (style) 
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/amfd/ckpt_enc.cc:2281] -> [src/amf/amfd/ckpt_enc.cc:2288]: (style) 
Variable 'status' is reassigned a value before the old one has been used.
[src/amf/amfd/ckpt_enc.cc:2314] -> [src/amf/amfd/ckpt_enc.cc:2322]: (style) 
Variable 'status' is reassigned a value before the old one has been used.
[src/amf/amfd/ckpt_enc.cc:1951]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:1982]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2015]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2044]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2076]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2111]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2151]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2176]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2216]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2252]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2470]: (performance) Prefer prefix ++/-- operators 
for non-primitive types.
[src/amf/amfd/clm.cc:452] -> [src/amf/amfd/clm.cc:456]: (style, inconclusive) 
Variable 'error' is reassigned a value before the old one has been used if 
variable is no semaphore variable.

[devel] [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues [#2341] V2

2017-03-14 Thread A V Mahesh
Summary:amf: Fix all Cppcheck 1.77 issues [#2341] V2 
Review request for Trac Ticket(s): #2341
Peer Reviewer(s): Amf Dev 
Pull request to: <>
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 servicesn
 OpenSAF servicesy
 Core libraries  n
 Samples n
 Tests   n
 Other   n


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

changeset 9401812b47ad5a079e6e82daad0ff83c1cf635f2
Author: A V Mahesh 
Date:   Wed, 15 Mar 2017 09:51:41 +0530

amfd: Fix all Cppcheck 1.77 issues [#2341] V2

V2 Re-based changes on OpenSAF  5.2.RC1 tagged code.

[src/amf/amfd/app.cc:285]: (style) The scope of the
variable 'i' can be reduced. [src/amf/amfd/apptype.cc:137]: (style)
Condition 'rc!=0' is always false [src/amf/amfd/apptype.cc:89] ->
[src/amf/amfd/apptype.cc:84]: (warning, inconclusive) Either the 
condition
'(attr=attributes[i++])!=nullptr' is redundant or there is possible null
pointer dereference: attr. [src/amf/amfd/apptype.cc:129] ->
[src/amf/amfd/apptype.cc:124]: (warning, inconclusive) Either the 
condition
'(attr=attributes[i++])!=nullptr' is redundant or there is possible null
pointer dereference: attr. [src/amf/amfd/apptype.cc:69]: (style) The 
scope
of the variable 'sg_type' can be reduced. [src/amf/amfd/chkop.cc:1297] 
->
[src/amf/amfd/chkop.cc:1302]: (style) Variable 'uba' is reassigned a 
value
before the old one has been used. [src/amf/amfd/ckpt_dec.cc:374] ->
[src/amf/amfd/ckpt_dec.cc:382]: (style) Variable 'status' is reassigned 
a
value before the old one has been used. [src/amf/amfd/ckpt_dec.cc:573] 
->
[src/amf/amfd/ckpt_dec.cc:577]: (style) Variable 'status' is reassigned 
a
value before the old one has been used. [src/amf/amfd/ckpt_dec.cc:1109]:
(performance) Prefer prefix ++/-- operators for non-primitive types.
[src/amf/amfd/ckpt_edu.cc:51] -> [src/amf/amfd/ckpt_edu.cc:56]: (style)
Variable 'rc' is reassigned a value before the old one has been used.
[src/amf/amfd/ckpt_enc.cc:2281] -> [src/amf/amfd/ckpt_enc.cc:2288]: 
(style)
Variable 'status' is reassigned a value before the old one has been 
used.
[src/amf/amfd/ckpt_enc.cc:2314] -> [src/amf/amfd/ckpt_enc.cc:2322]: 
(style)
Variable 'status' is reassigned a value before the old one has been 
used.
[src/amf/amfd/ckpt_enc.cc:1951]: (performance) Prefer prefix ++/-- 
operators
for non-primitive types. [src/amf/amfd/ckpt_enc.cc:1982]: (performance)
Prefer prefix ++/-- operators for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2015]: (performance) Prefer prefix ++/-- 
operators
for non-primitive types. [src/amf/amfd/ckpt_enc.cc:2044]: (performance)
Prefer prefix ++/-- operators for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2076]: (performance) Prefer prefix ++/-- 
operators
for non-primitive types. [src/amf/amfd/ckpt_enc.cc:2111]: (performance)
Prefer prefix ++/-- operators for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2151]: (performance) Prefer prefix ++/-- 
operators
for non-primitive types. [src/amf/amfd/ckpt_enc.cc:2176]: (performance)
Prefer prefix ++/-- operators for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2216]: (performance) Prefer prefix ++/-- 
operators
for non-primitive types. [src/amf/amfd/ckpt_enc.cc:2252]: (performance)
Prefer prefix ++/-- operators for non-primitive types.
[src/amf/amfd/ckpt_enc.cc:2470]: (performance) Prefer prefix ++/-- 
operators
for non-primitive types. [src/amf/amfd/clm.cc:452] ->
[src/amf/amfd/clm.cc:456]: (style, inconclusive) Variable 'error' is
reassigned a value before the old one has been used if variable is no
semaphore variable. [src/amf/amfd/clm.cc:473] -> 
[src/amf/amfd/clm.cc:475]:
(style, inconclusive) Variable 'error' is reassigned a value before the 
old
one has been used if variable is no semaphore variable.
[src/amf/amfd/clm.cc:344]: (performance) Prefer prefix ++/-- operators 
for
non-primitive types. [src/amf/amfd/cluster.cc:82]: (performance) Prefer
prefix ++/-- operators for non-primitive types.
[src/amf/amfd/cluster.cc:95]: (performance) Prefer prefix ++/-- 
operators
for non-primitive types. [src/amf/amfd/cluster.cc:116]: (performance) 
Prefer
prefix ++/-- operators for non-primitive types. 
[src/amf/amfd/comp.cc:1270]
-> [src/amf/amfd/comp.cc:1285]: (style) Variable 'su_node_ptr' is 

[devel] [PATCH 2 of 3] amfnd: Fix all Cppcheck 1.77 issues [#2341] V2

2017-03-14 Thread A V Mahesh
 src/amf/amfnd/amfnd.cc   |  15 +--
 src/amf/amfnd/cbq.cc |   9 +++--
 src/amf/amfnd/chc.cc |  29 +++--
 src/amf/amfnd/clc.cc |  37 ++---
 src/amf/amfnd/comp.cc|   8 +++-
 src/amf/amfnd/compdb.cc  |  17 -
 src/amf/amfnd/cpm.cc |  13 +
 src/amf/amfnd/di.cc  |  35 +--
 src/amf/amfnd/err.cc |  35 ++-
 src/amf/amfnd/evt.cc |   3 +--
 src/amf/amfnd/hcdb.cc|  13 +++--
 src/amf/amfnd/imm.cc |   5 ++---
 src/amf/amfnd/main.cc|  13 +
 src/amf/amfnd/mds.cc |  20 +++-
 src/amf/amfnd/pg.cc  |  33 +++--
 src/amf/amfnd/proxy.cc   |   8 +++-
 src/amf/amfnd/proxydb.cc |  17 +
 src/amf/amfnd/sidb.cc|   3 +--
 src/amf/amfnd/su.cc  |   3 +--
 src/amf/amfnd/sudb.cc|   5 ++---
 src/amf/amfnd/susm.cc|  19 +--
 src/amf/amfnd/util.cc|   2 +-
 22 files changed, 143 insertions(+), 199 deletions(-)


V2 Re-based changes on OpenSAF 5.2.RC1 tagged code.
[src/amf/amfnd/amfnd.cc:58] -> [src/amf/amfnd/amfnd.cc:63]: (style) Variable 
'del_cbk' is reassigned a value before the old one has been used.
[src/amf/amfnd/amfnd.cc:59] -> [src/amf/amfnd/amfnd.cc:65]: (style) Variable 
'o_comp' is reassigned a value before the old one has been used.
[src/amf/amfnd/amfnd.cc:191] -> [src/amf/amfnd/amfnd.cc:199]: (style) Variable 
'o_comp' is reassigned a value before the old one has been used.
[src/amf/amfnd/amfnd.cc:249] -> [src/amf/amfnd/amfnd.cc:250]: (style) Variable 
'res' is reassigned a value before the old one has been used.
[src/amf/amfnd/cbq.cc:441] -> [src/amf/amfnd/cbq.cc:442]: (style) Variable 
'temp_csi' is reassigned a value before the old one has been used.
[src/amf/amfnd/cbq.cc:734] -> [src/amf/amfnd/cbq.cc:748]: (style) Variable 'rc' 
is reassigned a value before the old one has been used.
[src/amf/amfnd/cbq.cc:1132] -> [src/amf/amfnd/cbq.cc:1133]: (style) Variable 
'temp_csi' is reassigned a value before the old one has been used.
[src/amf/amfnd/chc.cc:842]: (style) The scope of the variable 'cbk_rec' can be 
reduced.
[src/amf/amfnd/clc.cc:1402] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
redundant or there is possible null pointer dereference: curr_rec.
[src/amf/amfnd/clc.cc:1403] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
redundant or there is possible null pointer dereference: curr_rec.
[src/amf/amfnd/clc.cc:1404] -> [src/amf/amfnd/clc.cc:1408]: (warning, 
inconclusive) Either the condition 'NCSCC_RC_SUCCESS!=rc&_rec' is 
redundant or there is possible null pointer dereference: curr_rec.
[src/amf/amfnd/clc.cc:610] -> [src/amf/amfnd/clc.cc:616]: (style) Variable 
'clc_evt' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:1708] -> [src/amf/amfnd/clc.cc:1712]: (style) Variable 
'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:1749] -> [src/amf/amfnd/clc.cc:1753]: (style) Variable 
'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:1951] -> [src/amf/amfnd/clc.cc:1955]: (style) Variable 
'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:2370] -> [src/amf/amfnd/clc.cc:2376]: (style) Variable 
'csi' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:3052] -> [src/amf/amfnd/clc.cc:3053]: (style) Variable 
'tmp' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:3172] -> [src/amf/amfnd/clc.cc:3181]: (style) Variable 
'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/clc.cc:1356]: (style) The scope of the variable 'curr_rec' can 
be reduced.
[src/amf/amfnd/clc.cc:1384]: (style) The scope of the variable 'curr_rec' can 
be reduced.
[src/amf/amfnd/clc.cc:3042]: (style) The scope of the variable 'i' can be 
reduced.
[src/amf/amfnd/comp.cc:2557] -> [src/amf/amfnd/comp.cc:2561]: (warning) Either 
the condition 'if(csi&_AVND_COMP_IS_ALL_CSI(comp))' is redundant or there is 
possible null pointer dereference: csi.
[src/amf/amfnd/comp.cc:2237] -> [src/amf/amfnd/comp.cc:2250]: (style) Variable 
'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/comp.cc:2962] -> [src/amf/amfnd/comp.cc:2972]: (style) Variable 
'rc' is reassigned a value before the old one has been used.
[src/amf/amfnd/compdb.cc:485]: (warning) Comparison of a boolean expression 
with an integer.
[src/amf/amfnd/compdb.cc:486]: (warning) Comparison of a boolean expression 
with an integer.
[src/amf/amfnd/compdb.cc:485]: (warning) Comparison of a boolean value using 
relational operator (<, >, <= or >=).
[src/amf/amfnd/compdb.cc:732] -> [src/amf/amfnd/compdb.cc:734]: (warning) 
Either the 

Re: [devel] [PATCH 1 of 1] base: Improve trace by using tid instead of pid [#2370]

2017-03-14 Thread Anders Widell
Ack.

regards,

Anders Widell


On 03/13/2017 01:16 PM, Hans Nordeback wrote:
>   src/base/logtrace.c |  7 ++-
>   1 files changed, 6 insertions(+), 1 deletions(-)
>
>
> Even though not being LSB it would be good to use thread id instead of 
> process id in trace.
>
> diff --git a/src/base/logtrace.c b/src/base/logtrace.c
> --- a/src/base/logtrace.c
> +++ b/src/base/logtrace.c
> @@ -48,6 +48,11 @@ static const char *ident;
>   static const char *pathname;
>   static int logmask;
>   
> +static pid_t gettid(void)
> +{
> + return syscall(SYS_gettid);
> +}
> +
>   /**
>* USR2 signal handler to enable/disable trace (toggle)
>* @param sig
> @@ -100,7 +105,7 @@ void output_(const char *file, unsigned
>   i = snprintf(preamble, sizeof(preamble), "%s.%06ld %s ", log_string, 
> tv.tv_usec, ident);
>   
>   snprintf([i], sizeof(preamble) - i, "[%d:%s:%04u] %s %s",
> - getpid(), file, line, prefix_name[priority + category], format);
> + gettid(), file, line, prefix_name[priority + category], format);
>   i = vsnprintf(log_string, sizeof(log_string), preamble, ap);
>   
>   /* Check if the logtrace user had passed message length >= logtrace 
> array limit of 1023.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
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 build: Add missing README files to distribution tarball [#2374]

2017-03-14 Thread Anders Widell
Summary: build: Add missing README files to distribution tarball [#2374]
Review request for Trac Ticket(s): 2374
Peer Reviewer(s): Ramesh
Pull request to: 
Affected branch(es): default(5.2)
Development branch: default


Impacted area   Impact y/n

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


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

changeset 93ed88041a574a307567e89b3ddc039e8c2a6a50
Author: Anders Widell 
Date:   Tue, 14 Mar 2017 16:13:30 +0100

build: Add missing README files to distribution tarball [#2374]


Complete diffstat:
--
 python/Makefile.am   |  4 +++-
 src/amf/Makefile.am  |  9 -
 src/ckpt/Makefile.am |  7 ++-
 src/clm/Makefile.am  |  6 +-
 src/dtm/Makefile.am  |  3 +++
 src/imm/Makefile.am  |  4 +++-
 src/log/Makefile.am  |  8 +++-
 src/ntf/Makefile.am  |  8 +++-
 src/plm/Makefile.am  |  7 +--
 src/smf/Makefile.am  |  6 +-
 10 files changed, 52 insertions(+), 10 deletions(-)


Testing Commands:
-

make dist


Testing, Expected Results:
--

README files should be included in the OpenSAF distribution tarball.

Conditions of Submission:
-

Ack from reviewer(s).


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
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.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


[devel] [PATCH 1 of 1] build: Add missing README files to distribution tarball [#2374]

2017-03-14 Thread Anders Widell
 python/Makefile.am   |  4 +++-
 src/amf/Makefile.am  |  9 -
 src/ckpt/Makefile.am |  7 ++-
 src/clm/Makefile.am  |  6 +-
 src/dtm/Makefile.am  |  3 +++
 src/imm/Makefile.am  |  4 +++-
 src/log/Makefile.am  |  8 +++-
 src/ntf/Makefile.am  |  8 +++-
 src/plm/Makefile.am  |  7 +--
 src/smf/Makefile.am  |  6 +-
 10 files changed, 52 insertions(+), 10 deletions(-)


diff --git a/python/Makefile.am b/python/Makefile.am
--- a/python/Makefile.am
+++ b/python/Makefile.am
@@ -1,6 +1,7 @@
 #  -*- OpenSAF  -*-
 #
 # (C) Copyright 2011 The OpenSAF Foundation
+# Copyright Ericsson AB 2017 - All Rights Reserved.
 #
 # This program is distributed in the hope that it will be useful, but
 # WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -15,9 +16,10 @@
 #
 
 EXTRA_DIST += \
+   python/MANIFEST.in \
python/Makefile.mk \
-   python/MANIFEST.in \
python/README \
+   python/samples/README \
python/setup.py
 
 if ENABLE_PYTHON
diff --git a/src/amf/Makefile.am b/src/amf/Makefile.am
--- a/src/amf/Makefile.am
+++ b/src/amf/Makefile.am
@@ -1,6 +1,7 @@
 #  -*- OpenSAF  -*-
 #
 # (C) Copyright 2016 The OpenSAF Foundation
+# Copyright Ericsson AB 2017 - All Rights Reserved.
 #
 # This program is distributed in the hope that it will be useful, but
 # WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -14,6 +15,13 @@
 # Author(s): Ericsson AB
 #
 
+EXTRA_DIST += \
+   src/amf/README \
+   src/amf/README_LONGDN \
+   src/amf/README_NODEGROUP \
+   src/amf/README_SC_ABSENCE \
+   src/amf/saf/libSaAmf.map
+
 osaf_lib_LTLIBRARIES += lib/libamf_common.la
 
 lib_libamf_common_la_CFLAGS = $(OSAF_LIB_FLAGS) $(AM_CFLAGS)
@@ -155,7 +163,6 @@ noinst_HEADERS += \
 
 sbin_PROGRAMS += bin/amfpm
 osaf_execbin_PROGRAMS += bin/osafamfd bin/osafamfnd bin/osafamfwd
-EXTRA_DIST += src/amf/saf/libSaAmf.map
 CORE_INCLUDES += -I$(top_srcdir)/src/amf/saf
 TESTS += bin/testamfd
 pkgconfig_DATA += src/amf/saf/opensaf-amf.pc
diff --git a/src/ckpt/Makefile.am b/src/ckpt/Makefile.am
--- a/src/ckpt/Makefile.am
+++ b/src/ckpt/Makefile.am
@@ -1,6 +1,7 @@
 #  -*- OpenSAF  -*-
 #
 # (C) Copyright 2016 The OpenSAF Foundation
+# Copyright Ericsson AB 2017 - All Rights Reserved.
 #
 # This program is distributed in the hope that it will be useful, but
 # WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -14,7 +15,11 @@
 # Author(s): Ericsson AB
 #
 
-EXTRA_DIST += src/ckpt/saf/libSaCkpt.map
+EXTRA_DIST += \
+   src/ckpt/README.HEADLESS \
+   src/ckpt/README.LONGDN \
+   src/ckpt/apitest/README \
+   src/ckpt/saf/libSaCkpt.map
 
 if ENABLE_AIS_CKPT
 
diff --git a/src/clm/Makefile.am b/src/clm/Makefile.am
--- a/src/clm/Makefile.am
+++ b/src/clm/Makefile.am
@@ -1,6 +1,7 @@
 #  -*- OpenSAF  -*-
 #
 # (C) Copyright 2016 The OpenSAF Foundation
+# Copyright Ericsson AB 2017 - All Rights Reserved.
 #
 # This program is distributed in the hope that it will be useful, but
 # WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -14,6 +15,10 @@
 # Author(s): Ericsson AB
 #
 
+EXTRA_DIST += \
+   src/clm/README \
+   src/clm/saf/libSaClm.map
+
 osaf_lib_LTLIBRARIES += lib/libclm_common.la
 
 lib_libclm_common_la_CFLAGS = $(OSAF_LIB_FLAGS) $(AM_CFLAGS)
@@ -85,7 +90,6 @@ noinst_HEADERS += \
src/clm/common/clmsv_msg.h
 
 osaf_execbin_PROGRAMS += bin/osafclmd bin/osafclmna
-EXTRA_DIST += src/clm/saf/libSaClm.map
 CORE_INCLUDES += -I$(top_srcdir)/src/clm/saf
 pkgconfig_DATA += src/clm/saf/opensaf-clm.pc
 
diff --git a/src/dtm/Makefile.am b/src/dtm/Makefile.am
--- a/src/dtm/Makefile.am
+++ b/src/dtm/Makefile.am
@@ -15,6 +15,9 @@
 # Author(s): Ericsson AB
 #
 
+EXTRA_DIST += \
+   src/dtm/README
+
 noinst_HEADERS += \
src/dtm/dtmnd/dtm.h \
src/dtm/dtmnd/dtm_cb.h \
diff --git a/src/imm/Makefile.am b/src/imm/Makefile.am
--- a/src/imm/Makefile.am
+++ b/src/imm/Makefile.am
@@ -17,12 +17,14 @@
 #
 
 EXTRA_DIST += \
+   src/imm/README \
src/imm/README.2PBE \
src/imm/README.ACCESS_CONTROL \
src/imm/README.NO_DANGLING \
src/imm/README.RESOURCE_DISPLAY \
src/imm/README.SASTRINGT_API \
-   src/imm/README.SC_ABSENCE
+   src/imm/README.SC_ABSENCE \
+   src/imm/apitest/README
 
 osaf_lib_LTLIBRARIES += lib/libimm_common.la
 
diff --git a/src/log/Makefile.am b/src/log/Makefile.am
--- a/src/log/Makefile.am
+++ b/src/log/Makefile.am
@@ -15,7 +15,13 @@
 # Author(s): Ericsson AB
 #
 
-EXTRA_DIST += src/log/saf/libSaLog.map
+EXTRA_DIST += \
+   src/log/README \
+   src/log/README-HEADLESS \
+   src/log/README_LONGDN \
+   src/log/apitest/README \
+   src/log/saf/libSaLog.map \
+   src/log/tools/README
 
 lib_LTLIBRARIES += lib/libSaLog.la
 
diff --git a/src/ntf/Makefile.am b/src/ntf/Makefile.am
--- a/src/ntf/Makefile.am
+++ b/src/ntf/Makefile.am
@@ -1,6 +1,7 @@
 #  

[devel] OpenSAF 5.2.RC1 tagged

2017-03-14 Thread Anders Widell
Hi all!

We have now tagged and released OpenSAF 5.2.RC1 (release candidate 1), and we 
are planning for a second release candidate within the next one or two weeks. 
After the second release candidate (5.2.RC2) has been tagged, all branches in 
the Mercurial repository will be under change control. This means that only 
approved bug fixes may be pushed between 5.2.RC2 and the final 5.2 release.

Please go through the list of open tickets on the 5.2.RC2 milestone and try to 
close as many of the tickets as possible before 5.2.RC2 is tagged:

https://sourceforge.net/p/opensaf/tickets/search/?q=status%3A%28accepted+assigned+unassigned+review%29+AND+_milestone%3A%285.2.FC+5.2.RC1+5.2.RC2%29=100

Please also check if some documentation needs to be updated for any of the 
enhancements you have implemented in the OpenSAF 5.2 release. The full list of 
5.2 enhancement tickets can be listed using the following link:

https://sourceforge.net/p/opensaf/tickets/search/?q=status%3A%28accepted+review+fixed%29+AND+_milestone%3A%285.2.FC+5.2.RC1+5.2.RC2+5.2.0%29+AND+_type%3Aenhancement=100

thanks,
Anders Widell


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


[devel] [PATCH 1 of 1] base: Cache the FQDN to avoid problems caused by slow DNS responses [#2347]

2017-03-14 Thread Anders Widell
 src/base/conf.cc|  66 +---
 src/base/conf.h |   5 
 src/nid/nodeinit.cc |   5 
 3 files changed, 62 insertions(+), 14 deletions(-)


Read the fully qualified domain name once and store it in the local file system.
This will solve problems with slow DNS responses, since the DNS system will only
be queried once.

diff --git a/src/base/conf.cc b/src/base/conf.cc
--- a/src/base/conf.cc
+++ b/src/base/conf.cc
@@ -56,7 +56,14 @@ void Conf::InitFullyQualifiedDomainName(
   Lock lock(i->mutex_);
   if (i->fully_qualified_domain_name_initialized_ == false) {
 i->fully_qualified_domain_name_ =
-GetFullyQualifiedDomainName(i->short_host_name_);
+ReadFile(PKGLOCALSTATEDIR "/fully_qualified_domain_name",
+ NI_MAXHOST - 1, "");
+if (i->fully_qualified_domain_name_.empty()) {
+  i->fully_qualified_domain_name_ =
+  GetFullyQualifiedDomainName(i->short_host_name_);
+  WriteFileAtomically(PKGLOCALSTATEDIR "/fully_qualified_domain_name",
+i->fully_qualified_domain_name_);
+}
 i->fully_qualified_domain_name_initialized_ = true;
   }
 }
@@ -114,10 +121,12 @@ std::string Conf::GetFullyQualifiedDomai
  host, sizeof(host), nullptr, 0, 0);
 if (rc == 0) {
   TRACE("getnameinfo() successful, hostname='%s'", host);
-  if (strncmp(short_host_name.c_str(), host, short_host_name.size()) 
== 0 &&
-  host[short_host_name.size()] == '.') {
+  std::string::size_type size = short_host_name.size();
+  if (size == 0 ||
+  (strncmp(short_host_name.c_str(), host, size) == 0 &&
+   host[size] == '.')) {
 fqdn = host;
-break;
+if (strchr(host, '.') != nullptr) break;
   }
 } else {
   TRACE("getnameinfo() failed with error %d", rc);
@@ -132,22 +141,15 @@ std::string Conf::GetFullyQualifiedDomai
 }
 
 std::string Conf::GetNodeName(const std::string& short_host_name) {
-  std::string node_name;
-  std::ifstream str;
-  str.width(255);
-  try {
-str.open(PKGSYSCONFDIR "/node_name");
-str >> node_name;
-  } catch (std::ifstream::failure) {
-node_name.clear();
-  }
-  return (str.fail() || node_name.empty()) ? short_host_name : node_name;
+  return ReadFile(PKGSYSCONFDIR "/node_name", 255, short_host_name);
 }
 
 std::string Conf::GetShortHostName() {
   char short_host_name[HOST_NAME_MAX + 1];
   if (gethostname(short_host_name, sizeof(short_host_name)) == 0) {
 short_host_name[sizeof(short_host_name) - 1] = '\0';
+char* dot = strchr(short_host_name, '.');
+if (dot != nullptr) *dot = '\0';
   } else {
 LOG_ER("gethostname() failed, errno=%d", errno);
 short_host_name[0] = '\0';
@@ -155,4 +157,40 @@ std::string Conf::GetShortHostName() {
   return short_host_name;
 }
 
+std::string Conf::ReadFile(const std::string& path_name,
+   std::string::size_type max_length,
+   const std::string& default_contents) {
+  std::string contents;
+  std::ifstream str;
+  str.width(max_length);
+  try {
+str.open(path_name);
+str >> contents;
+  } catch (std::ifstream::failure) {
+contents.clear();
+  }
+  return (str.fail() || contents.empty()) ? default_contents : contents;
+}
+
+void Conf::WriteFileAtomically(const std::string& path_name,
+   const std::string& contents) {
+  std::string tmp_file = path_name + "." + std::to_string(getpid());
+  std::ofstream str;
+  bool success = true;
+  try {
+str.open(tmp_file, std::ofstream::out | std::ofstream::trunc);
+str << contents << std::endl;
+  } catch (std::ofstream::failure) {
+success = false;
+  }
+  str.close();
+  if (success && !str.fail()) {
+if (link(tmp_file.c_str(), path_name.c_str()) != 0) {
+  TRACE("link('%s', '%s') failed with errno %d", tmp_file.c_str(),
+path_name.c_str(), errno);
+}
+  }
+  unlink(tmp_file.c_str());
+}
+
 }  // namespace base
diff --git a/src/base/conf.h b/src/base/conf.h
--- a/src/base/conf.h
+++ b/src/base/conf.h
@@ -88,6 +88,11 @@ class Conf {
   const std::string& short_host_name);
   static std::string GetNodeName(const std::string& short_host_name);
   static std::string GetShortHostName();
+  static std::string ReadFile(const std::string& path_name,
+   std::string::size_type max_length,
+   const std::string& default_contents);
+  static void WriteFileAtomically(const std::string& path_name,
+  const std::string& contents);
   static pthread_once_t once_control_;
   static Conf* instance_;
   std::string fully_qualified_domain_name_;
diff --git a/src/nid/nodeinit.cc b/src/nid/nodeinit.cc
--- a/src/nid/nodeinit.cc
+++ b/src/nid/nodeinit.cc
@@ -70,6 +70,7 @@
 #include 
 #include 
 
+#include "base/conf.h"
 #include "base/osaf_poll.h"
 

[devel] [PATCH 0 of 1] Review Request for base: Cache the FQDN to avoid problems caused by slow DNS responses [#2347]

2017-03-14 Thread Anders Widell
Summary: base: Cache the FQDN to avoid problems caused by slow DNS responses 
[#2347]
Review request for Trac Ticket(s): 2347
Peer Reviewer(s): Ramesh
Pull request to: 
Affected branch(es): default(5.2)
Development branch: default


Impacted area   Impact y/n

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


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

changeset 3a44e8502e4343de1ecd5722a453e58f21c4364c
Author: Anders Widell 
Date:   Tue, 14 Mar 2017 12:31:17 +0100

base: Cache the FQDN to avoid problems caused by slow DNS responses 
[#2347]

Read the fully qualified domain name once and store it in the local file
system. This will solve problems with slow DNS responses, since the DNS
system will only be queried once.


Complete diffstat:
--
 src/base/conf.cc|  66 
--
 src/base/conf.h |   5 +
 src/nid/nodeinit.cc |   5 +
 3 files changed, 62 insertions(+), 14 deletions(-)


Testing Commands:
-

Start OpenSAF on a system with a slow or misconfigured DNS server.


Testing, Expected Results:
--

OpenSAF should start successfully.


Conditions of Submission:
-

Ack from reviewer(s)


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
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.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


[devel] [PATCH 1 of 1] amfd: handle BAD_HANDLE return during config read [#2361]

2017-03-14 Thread nagendra . k
 osaf/services/saf/amf/amfd/imm.cc  |  18 +-
 osaf/services/saf/amf/amfd/role.cc |   4 ++--
 2 files changed, 15 insertions(+), 7 deletions(-)


If Immnd is killed, Amfd re-initializes with imm and re-reads configuration.
During configuration read if Immnd is again killed, Amfd exits.
The fix is to re-initialize if configuration reading fails.
Also, the fix re-initializes Om handles in case it is stale.

diff --git a/osaf/services/saf/amf/amfd/imm.cc 
b/osaf/services/saf/amf/amfd/imm.cc
--- a/osaf/services/saf/amf/amfd/imm.cc
+++ b/osaf/services/saf/amf/amfd/imm.cc
@@ -1663,7 +1663,7 @@ done:
if (rc == NCSCC_RC_SUCCESS)
TRACE("AMF Configuration successfully read from IMM");
else
-   LOG_ER("Failed to read configuration, AMF will not start");
+   LOG_WA("Failed to read configuration.");
 
TRACE_LEAVE2("%u", rc);
return rc;
@@ -1964,11 +1964,19 @@ static void *avd_imm_reinit_bg_thread(vo
osaf_mutex_unlock_ordie(_reinit_mutex);
exit(EXIT_FAILURE);
}
-
+   /* Lets re-initialize Om interface also. */
+   (void) immutil_saImmOmFinalize(cb->immOmHandle);
+   if ((rc = immutil_saImmOmInitialize(>immOmHandle, 
nullptr, )) != SA_AIS_OK) {
+   LOG_ER("saImmOmInitialize failed %u", rc);
+   continue;
+   }
if (avd_imm_config_get() != NCSCC_RC_SUCCESS) {
-   LOG_ER("avd_imm_config_get FAILED");
-   osaf_mutex_unlock_ordie(_reinit_mutex);
-   exit(EXIT_FAILURE);
+   /* This can come when Immnd is killed again 
during
+  config read and reading the config returned 
BAD_HANDLE.
+  In other return types also, it is good to 
retry.
+  In normal situations, retry will help. */
+   LOG_WA("avd_imm_config_get FAILED");
+   continue;
}
}
break;
diff --git a/osaf/services/saf/amf/amfd/role.cc 
b/osaf/services/saf/amf/amfd/role.cc
--- a/osaf/services/saf/amf/amfd/role.cc
+++ b/osaf/services/saf/amf/amfd/role.cc
@@ -270,7 +270,7 @@ uint32_t avd_active_role_initialization(
 }
 
if (avd_imm_config_get() != NCSCC_RC_SUCCESS) {
-   LOG_ER("avd_imm_config_get FAILED");
+   LOG_ER("avd_imm_config_get FAILED, AMF will not start.");
goto done;
}
 
@@ -304,7 +304,7 @@ uint32_t avd_standby_role_initialization
 }
 
if (avd_imm_config_get() != NCSCC_RC_SUCCESS) {
-   LOG_ER("avd_imm_config_get FAILED");
+   LOG_ER("avd_imm_config_get FAILED, AMF will not start.");
goto done;
}
 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
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 amfd: handle BAD_HANDLE return during config read [#2361]

2017-03-14 Thread nagendra . k
Summary: amfd: handle BAD_HANDLE return during config read [#2361]
Review request for Trac Ticket(s): #2361
Peer Reviewer(s): Amf Dev
Pull request to: <>
Affected branch(es): All
Development branch: opensaf-5.1.x


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 de01cc39a7252dd01ddd572731f95776742082a1
Author: Nagendra Kumar
Date:   Tue, 14 Mar 2017 15:20:07 +0530

amfd: handle BAD_HANDLE return during config read [#2361] If Immnd is
killed, Amfd re-initializes with imm and re-reads configuration. During
configuration read if Immnd is again killed, Amfd exits. The fix is to 
re-
initialize if configuration reading fails. Also, the fix re-initializes 
Om
handles in case it is stale.


Complete diffstat:
--
 osaf/services/saf/amf/amfd/imm.cc  |  18 +-
 osaf/services/saf/amf/amfd/role.cc |   4 ++--
 2 files changed, 15 insertions(+), 7 deletions(-)


Testing Commands:
-
As per ticket description dated: 14 March 2017.

Testing, Expected Results:
--
Amfd should re-initialize itself.

Conditions of Submission:
-
Ack.

Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
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.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
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 logd crash on Active side [#2362]

2017-03-14 Thread Vu Minh Nguyen
Ack with minor comment, see [Vu].

Regards, Vu

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: Monday, March 13, 2017 1:19 PM
> To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au;
> mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 1] log: fix logd crash on Active side [#2362]
> 
>  src/log/logd/lgs_filehdl.cc |  21 +
>  1 files changed, 13 insertions(+), 8 deletions(-)
> 
> 
> The cause of issue is free cfg_namelist while struct dirent **cfg_namelist
> unallocated
> 
> diff --git a/src/log/logd/lgs_filehdl.cc b/src/log/logd/lgs_filehdl.cc
> --- a/src/log/logd/lgs_filehdl.cc
> +++ b/src/log/logd/lgs_filehdl.cc
> @@ -1,6 +1,7 @@
>  /*  -*- OpenSAF  -*-
>   *
>   * (C) Copyright 2013 The OpenSAF Foundation
> + * Copyright Ericsson AB 2013, 2017 - All Rights Reserved.
>   *
>   * This program is distributed in the hope that it will be useful, but
>   * WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> @@ -704,8 +705,8 @@ done_exit:
>   * @return int, number of cfgfiles or -1 if error
>   */
>  int get_number_of_cfg_files_hdl(void *indata, void *outdata, size_t
> max_outsize) {
> -  struct dirent **cfg_namelist;
> -  struct dirent **log_namelist;
> +  struct dirent **cfg_namelist = nullptr;
> +  struct dirent **log_namelist = nullptr;
>int n, cfg_old_date = -1, cfg_old_time = -1, old_ind = -1, cfg_files =
0, i,
> failed = 0;
>int log_old_date = -1, log_old_time = -1 , log_files = 0;
>std::string path;
> @@ -802,15 +803,19 @@ int get_number_of_cfg_files_hdl(void *in
> 
>  done_cfg_free:
>/* Free scandir allocated memory */
> -  for (i = 0; i < cfg_files; i++)
> -free(cfg_namelist[i]);
> -  free(cfg_namelist);
> +  if (cfg_namelist != NULL) {
[Vu] Use nullptr instead. Seems free() does nothing if pointer is nullptr,
if so no need to check null those?
> +for (i = 0; i < cfg_files; i++)
> +  free(cfg_namelist[i]);
> +free(cfg_namelist);
> +  }
> 
>  done_log_free:
>/* Free scandir allocated memory */
> -  for (i = 0; i < log_files; i++)
> -free(log_namelist[i]);
> -  free(log_namelist);
> +  if (log_namelist != NULL) {
[Vu] Same comment as above.
> +for (i = 0; i < log_files; i++)
> +  free(log_namelist[i]);
> +free(log_namelist);
> +  }
> 
>  done_exit:
>TRACE_LEAVE();


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 1 of 1] fm: changing the log level from ER to WA [#2363]

2017-03-14 Thread praveen malviya
Ack, code review only.

Thanks,
Praveen

On 14-Mar-17 11:54 AM, ramesh.bet...@oracle.com wrote:
>  src/fm/fmd/fm_main.c |  2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
>
> diff --git a/src/fm/fmd/fm_main.c b/src/fm/fmd/fm_main.c
> --- a/src/fm/fmd/fm_main.c
> +++ b/src/fm/fmd/fm_main.c
> @@ -608,7 +608,7 @@ static void fm_mbx_msg_handler(FM_CB *fm
>* (old-Active) is still in the progress of shutdown (i.e., 
> amfd/immd is still alive).
>*/
>   if ((fm_cb->role == PCS_RDA_ACTIVE) && (fm_cb->csi_assigned == 
> false)) {
> - LOG_ER("Two active controllers observed in a cluster, 
> newActive: %x and old-Active: %x", fm_cb->node_id, fm_cb->peer_node_id);
> + LOG_WA("Two active controllers observed in a cluster, 
> newActive: %x and old-Active: %x", fm_cb->node_id, fm_cb->peer_node_id);
>   opensaf_reboot(0, NULL,
>   "Received svc up from peer node (old-active is not 
> fully DOWN), hence rebooting the new Active");
>   }
>

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
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 fm: changing the log level from ER to WA [#2363]

2017-03-14 Thread ramesh . betham
Summary: fm: changing the log level from ER to WA [#2363]
Review request for Trac Ticket(s): 2363
Peer Reviewer(s): praveen
Affected branch(es): default(5.2)
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 d2caf6367b19ebabf8007264968ad646b69af0b4
Author: Ramesh Betham
Date:   Tue, 14 Mar 2017 11:51:01 +0530

fm: changing the log level from ER to WA [#2363]


Complete diffstat:
--
 src/fm/fmd/fm_main.c |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


Testing Commands:
-


Testing, Expected Results:
--


Conditions of Submission:
-
Ack from Reviewer

Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
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.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


[devel] [PATCH 1 of 1] fm: changing the log level from ER to WA [#2363]

2017-03-14 Thread ramesh . betham
 src/fm/fmd/fm_main.c |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/src/fm/fmd/fm_main.c b/src/fm/fmd/fm_main.c
--- a/src/fm/fmd/fm_main.c
+++ b/src/fm/fmd/fm_main.c
@@ -608,7 +608,7 @@ static void fm_mbx_msg_handler(FM_CB *fm
 * (old-Active) is still in the progress of shutdown (i.e., 
amfd/immd is still alive). 
 */
if ((fm_cb->role == PCS_RDA_ACTIVE) && (fm_cb->csi_assigned == 
false)) {
-   LOG_ER("Two active controllers observed in a cluster, 
newActive: %x and old-Active: %x", fm_cb->node_id, fm_cb->peer_node_id);
+   LOG_WA("Two active controllers observed in a cluster, 
newActive: %x and old-Active: %x", fm_cb->node_id, fm_cb->peer_node_id);
opensaf_reboot(0, NULL,
"Received svc up from peer node (old-active is not 
fully DOWN), hence rebooting the new Active");
}

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues [#2341] V1

2017-03-14 Thread Gary Lee
Hi Mahesh

Perhaps it's easier if you pushed V1 first. Otherwise the patches get even 
bigger and harder to review. I was referring to regression tests failing 
without the changes I proposed, when I said legacy tests failed.

thanks

> On 14 Mar 2017, at 6:01 pm, A V Mahesh  wrote:
> 
> Hi Gary,
> 
> Previously you found some old application issue and you resolved it is that 
> related to this path or different issue ?
> 
> -AVM
> 
> 
>> On 3/14/2017 12:20 PM, A V Mahesh wrote:
>> Hi Gar,
>> 
>> Thanks for the review.
>> 
>>> On 3/14/2017 11:47 AM, Gary Lee wrote:
>>> By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, 
>>> but this is a great improvement.
>> Ok  will re-run the  Cppcheck and if we find considerable , I will
>> re-publish the V2 patch.
>> 
>> -AVM
>> 
>>> On 3/14/2017 11:47 AM, Gary Lee wrote:
>>> Hi Mahesh
>>> 
>>> Ack for the series (regression tests run) with the following changes.
>>> 
>>> By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, 
>>> but this is a great improvement.
>>> 
>>> Thanks
>>> Gary
>>> 
>>> diff --git a/src/amf/amfd/csi.cc b/src/amf/amfd/csi.cc
>>> --- a/src/amf/amfd/csi.cc
>>> +++ b/src/amf/amfd/csi.cc
>>> @@ -1399,12 +1399,13 @@ void avd_csi_remove_csiattr(AVD_CSI *csi
>>>  void avd_csi_add_csiattr(AVD_CSI *csi, AVD_CSI_ATTR *csiattr)
>>>   {
>>> -int cnt = 0;
>>> +int cnt = 1;
>>>   AVD_CSI_ATTR *ptr;
>>>  TRACE_ENTER();
>>>   /* Count number of attributes (multivalue) */
>>>   ptr = csiattr;
>>> +osafassert(ptr != nullptr);
>>>   while (ptr->attr_next != nullptr) {
>>>   cnt++;
>>>   ptr = ptr->attr_next;
>>> diff --git a/src/amf/amfnd/cpm.cc b/src/amf/amfnd/cpm.cc
>>> --- a/src/amf/amfnd/cpm.cc
>>> +++ b/src/amf/amfnd/cpm.cc
>>> @@ -145,7 +145,7 @@ void avnd_comp_pm_rec_del(AVND_CB *cb, A
>>>   LOG_NO("PM Rec doesn't exist in Comp '%s' of pid %llu", 
>>> comp->name.c_str(), pid);
>>>   }
>>>   -delete rec;/* rec is no more, dont use it */
>>> +rec = nullptr;/* rec is no more, dont use it */
>>>  /* remove the corresponding element from mon_req list */
>>>   rc = avnd_mon_req_del(cb, pid);
>>> 
>>> 
>>> -Original Message-
>>> From: A V Mahesh 
>>> Date: Wednesday, 8 March 2017 at 11:28 pm
>>> To: , gary , 
>>> , 
>>> Cc: 
>>> Subject: [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 
>>> issues [#2341] V1
>>> 
>>>  Summary:amf: Fix all Cppcheck 1.77 issues [#2341] V1
>>>  Review request for Trac Ticket(s): #2341
>>>  Peer Reviewer(s): Amf Dev
>>>  Pull request to: <>
>>>  Affected branch(es): default, 5.2
>>>  Development branch: default
>>>
>>>  Impacted area   Impact y/n
>>>  
>>>   Docsn
>>>   Build systemn
>>>   RPM/packaging   n
>>>   Configuration files n
>>>   Startup scripts n
>>>   SAF servicesn
>>>   OpenSAF servicesy
>>>   Core libraries  n
>>>   Samples n
>>>   Tests   n
>>>   Other   n
>>>  Comments (indicate scope for each "y" above):
>>>  -
>>>changeset b8e7593cf4eadaa6169fe635ac0df67560ea875a
>>>  Author:A V Mahesh 
>>>  Date:Wed, 08 Mar 2017 17:52:21 +0530
>>>amfd: Fix all Cppcheck 1.77 issues [#2341] V1
>>>[staging/src/amf/amfd/app.cc:285]: (style) The scope of the 
>>> variable 'i' can
>>>  be reduced. [staging/src/amf/amfd/apptype.cc:137]: (style) 
>>> Condition 'rc!=0'
>>>  is always false [staging/src/amf/amfd/apptype.cc:69]: (style) The 
>>> scope of
>>>  the variable 'sg_type' can be reduced. 
>>> [staging/src/amf/amfd/chkop.cc:1297]
>>>  -> [staging/src/amf/amfd/chkop.cc:1302]: (style) Variable 'uba' is
>>>  reassigned a value before the old one has been used.
>>>  [staging/src/amf/amfd/ckpt_dec.cc:374] ->
>>>  [staging/src/amf/amfd/ckpt_dec.cc:382]: (style) Variable 'status' 
>>> is
>>>  reassigned a value before the old one has been used.
>>>  [staging/src/amf/amfd/ckpt_dec.cc:573] ->
>>>  [staging/src/amf/amfd/ckpt_dec.cc:577]: (style) Variable 'status' 
>>> is
>>>  reassigned a value before the old one has been used.
>>>  [staging/src/amf/amfd/ckpt_dec.cc:1109]: (performance) Prefer 
>>> prefix ++/--
>>>  operators for non-primitive types. 
>>> [staging/src/amf/amfd/ckpt_edu.cc:51] ->
>>>  [staging/src/amf/amfd/ckpt_edu.cc:56]: (style) Variable 

Re: [devel] [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues [#2341] V1

2017-03-14 Thread A V Mahesh
Hi Gary,

Previously you found some old application issue and you resolved it is 
that related to this path or different issue ?

-AVM


On 3/14/2017 12:20 PM, A V Mahesh wrote:
> Hi Gar,
>
> Thanks for the review.
>
> On 3/14/2017 11:47 AM, Gary Lee wrote:
>> By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, 
>> but this is a great improvement.
> Ok  will re-run the  Cppcheck and if we find considerable , I will
> re-publish the V2 patch.
>
> -AVM
>
> On 3/14/2017 11:47 AM, Gary Lee wrote:
>> Hi Mahesh
>>
>> Ack for the series (regression tests run) with the following changes.
>>
>> By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, 
>> but this is a great improvement.
>>
>> Thanks
>> Gary
>>
>> diff --git a/src/amf/amfd/csi.cc b/src/amf/amfd/csi.cc
>> --- a/src/amf/amfd/csi.cc
>> +++ b/src/amf/amfd/csi.cc
>> @@ -1399,12 +1399,13 @@ void avd_csi_remove_csiattr(AVD_CSI *csi
>>
>>void avd_csi_add_csiattr(AVD_CSI *csi, AVD_CSI_ATTR *csiattr)
>>{
>> -int cnt = 0;
>> +int cnt = 1;
>>  AVD_CSI_ATTR *ptr;
>>
>>  TRACE_ENTER();
>>  /* Count number of attributes (multivalue) */
>>  ptr = csiattr;
>> +osafassert(ptr != nullptr);
>>  while (ptr->attr_next != nullptr) {
>>  cnt++;
>>  ptr = ptr->attr_next;
>> diff --git a/src/amf/amfnd/cpm.cc b/src/amf/amfnd/cpm.cc
>> --- a/src/amf/amfnd/cpm.cc
>> +++ b/src/amf/amfnd/cpm.cc
>> @@ -145,7 +145,7 @@ void avnd_comp_pm_rec_del(AVND_CB *cb, A
>>  LOG_NO("PM Rec doesn't exist in Comp '%s' of pid %llu", 
>> comp->name.c_str(), pid);
>>  }
>>
>> -delete rec; /* rec is no more, dont use it */
>> +rec = nullptr;  /* rec is no more, dont use it */
>>
>>  /* remove the corresponding element from mon_req list */
>>  rc = avnd_mon_req_del(cb, pid);
>>
>>
>> -Original Message-
>> From: A V Mahesh 
>> Date: Wednesday, 8 March 2017 at 11:28 pm
>> To: , gary , 
>> , 
>> Cc: 
>> Subject: [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues 
>> [#2341] V1
>>
>>   Summary:amf: Fix all Cppcheck 1.77 issues [#2341] V1
>>   Review request for Trac Ticket(s): #2341
>>   Peer Reviewer(s): Amf Dev
>>   Pull request to: <>
>>   Affected branch(es): default, 5.2
>>   Development branch: default
>>   
>>   
>>   Impacted area   Impact y/n
>>   
>>Docsn
>>Build systemn
>>RPM/packaging   n
>>Configuration files n
>>Startup scripts n
>>SAF servicesn
>>OpenSAF servicesy
>>Core libraries  n
>>Samples n
>>Tests   n
>>Other   n
>>   
>>   
>>   Comments (indicate scope for each "y" above):
>>   -
>>   
>>   changeset b8e7593cf4eadaa6169fe635ac0df67560ea875a
>>   Author:A V Mahesh 
>>   Date:  Wed, 08 Mar 2017 17:52:21 +0530
>>   
>>  amfd: Fix all Cppcheck 1.77 issues [#2341] V1
>>   
>>  [staging/src/amf/amfd/app.cc:285]: (style) The scope of the 
>> variable 'i' can
>>  be reduced. [staging/src/amf/amfd/apptype.cc:137]: (style) 
>> Condition 'rc!=0'
>>  is always false [staging/src/amf/amfd/apptype.cc:69]: (style) 
>> The scope of
>>  the variable 'sg_type' can be reduced. 
>> [staging/src/amf/amfd/chkop.cc:1297]
>>  -> [staging/src/amf/amfd/chkop.cc:1302]: (style) Variable 'uba' 
>> is
>>  reassigned a value before the old one has been used.
>>  [staging/src/amf/amfd/ckpt_dec.cc:374] ->
>>  [staging/src/amf/amfd/ckpt_dec.cc:382]: (style) Variable 
>> 'status' is
>>  reassigned a value before the old one has been used.
>>  [staging/src/amf/amfd/ckpt_dec.cc:573] ->
>>  [staging/src/amf/amfd/ckpt_dec.cc:577]: (style) Variable 
>> 'status' is
>>  reassigned a value before the old one has been used.
>>  [staging/src/amf/amfd/ckpt_dec.cc:1109]: (performance) Prefer 
>> prefix ++/--
>>  operators for non-primitive types. 
>> [staging/src/amf/amfd/ckpt_edu.cc:51] ->
>>  [staging/src/amf/amfd/ckpt_edu.cc:56]: (style) Variable 'rc' is 
>> reassigned a
>>  value before the old one has been used.
>>  [staging/src/amf/amfd/ckpt_enc.cc:2281] ->
>>  [staging/src/amf/amfd/ckpt_enc.cc:2288]: (style) Variable 
>> 'status' is
>>  reassigned a value before the old one has been used.
>>   

Re: [devel] [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues [#2341] V1

2017-03-14 Thread A V Mahesh
Hi Gar,

Thanks for the review.

On 3/14/2017 11:47 AM, Gary Lee wrote:
> By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, 
> but this is a great improvement.

Ok  will re-run the  Cppcheck and if we find considerable , I will 
re-publish the V2 patch.

-AVM

On 3/14/2017 11:47 AM, Gary Lee wrote:
> Hi Mahesh
>
> Ack for the series (regression tests run) with the following changes.
>
> By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, 
> but this is a great improvement.
>
> Thanks
> Gary
>
> diff --git a/src/amf/amfd/csi.cc b/src/amf/amfd/csi.cc
> --- a/src/amf/amfd/csi.cc
> +++ b/src/amf/amfd/csi.cc
> @@ -1399,12 +1399,13 @@ void avd_csi_remove_csiattr(AVD_CSI *csi
>   
>   void avd_csi_add_csiattr(AVD_CSI *csi, AVD_CSI_ATTR *csiattr)
>   {
> - int cnt = 0;
> + int cnt = 1;
>   AVD_CSI_ATTR *ptr;
>   
>   TRACE_ENTER();
>   /* Count number of attributes (multivalue) */
>   ptr = csiattr;
> + osafassert(ptr != nullptr);
>   while (ptr->attr_next != nullptr) {
>   cnt++;
>   ptr = ptr->attr_next;
> diff --git a/src/amf/amfnd/cpm.cc b/src/amf/amfnd/cpm.cc
> --- a/src/amf/amfnd/cpm.cc
> +++ b/src/amf/amfnd/cpm.cc
> @@ -145,7 +145,7 @@ void avnd_comp_pm_rec_del(AVND_CB *cb, A
>   LOG_NO("PM Rec doesn't exist in Comp '%s' of pid %llu", 
> comp->name.c_str(), pid);
>   }
>   
> - delete rec; /* rec is no more, dont use it */
> + rec = nullptr;  /* rec is no more, dont use it */
>   
>   /* remove the corresponding element from mon_req list */
>   rc = avnd_mon_req_del(cb, pid);
>
>
> -Original Message-
> From: A V Mahesh 
> Date: Wednesday, 8 March 2017 at 11:28 pm
> To: , gary , 
> , 
> Cc: 
> Subject: [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues 
> [#2341] V1
>
>  Summary:amf: Fix all Cppcheck 1.77 issues [#2341] V1
>  Review request for Trac Ticket(s): #2341
>  Peer Reviewer(s): Amf Dev
>  Pull request to: <>
>  Affected branch(es): default, 5.2
>  Development branch: default
>  
>  
>  Impacted area   Impact y/n
>  
>   Docsn
>   Build systemn
>   RPM/packaging   n
>   Configuration files n
>   Startup scripts n
>   SAF servicesn
>   OpenSAF servicesy
>   Core libraries  n
>   Samples n
>   Tests   n
>   Other   n
>  
>  
>  Comments (indicate scope for each "y" above):
>  -
>  
>  changeset b8e7593cf4eadaa6169fe635ac0df67560ea875a
>  Author:  A V Mahesh 
>  Date:Wed, 08 Mar 2017 17:52:21 +0530
>  
>   amfd: Fix all Cppcheck 1.77 issues [#2341] V1
>  
>   [staging/src/amf/amfd/app.cc:285]: (style) The scope of the variable 
> 'i' can
>   be reduced. [staging/src/amf/amfd/apptype.cc:137]: (style) Condition 
> 'rc!=0'
>   is always false [staging/src/amf/amfd/apptype.cc:69]: (style) The scope 
> of
>   the variable 'sg_type' can be reduced. 
> [staging/src/amf/amfd/chkop.cc:1297]
>   -> [staging/src/amf/amfd/chkop.cc:1302]: (style) Variable 'uba' is
>   reassigned a value before the old one has been used.
>   [staging/src/amf/amfd/ckpt_dec.cc:374] ->
>   [staging/src/amf/amfd/ckpt_dec.cc:382]: (style) Variable 'status' is
>   reassigned a value before the old one has been used.
>   [staging/src/amf/amfd/ckpt_dec.cc:573] ->
>   [staging/src/amf/amfd/ckpt_dec.cc:577]: (style) Variable 'status' is
>   reassigned a value before the old one has been used.
>   [staging/src/amf/amfd/ckpt_dec.cc:1109]: (performance) Prefer prefix 
> ++/--
>   operators for non-primitive types. 
> [staging/src/amf/amfd/ckpt_edu.cc:51] ->
>   [staging/src/amf/amfd/ckpt_edu.cc:56]: (style) Variable 'rc' is 
> reassigned a
>   value before the old one has been used.
>   [staging/src/amf/amfd/ckpt_enc.cc:2281] ->
>   [staging/src/amf/amfd/ckpt_enc.cc:2288]: (style) Variable 'status' is
>   reassigned a value before the old one has been used.
>   [staging/src/amf/amfd/ckpt_enc.cc:2314] ->
>   [staging/src/amf/amfd/ckpt_enc.cc:2322]: (style) Variable 'status' is
>   reassigned a value before the old one has been used.
>   [staging/src/amf/amfd/ckpt_enc.cc:1951]: (performance) Prefer prefix 
> ++/--
>   operators for non-primitive types. 
> [staging/src/amf/amfd/ckpt_enc.cc:1982]:
>   (performance) Prefer prefix ++/-- operators for non-primitive types.
>   [staging/src/amf/amfd/ckpt_enc.cc:2015]: 

Re: [devel] [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues [#2341] V1

2017-03-14 Thread Gary Lee
Hi Mahesh

Ack for the series (regression tests run) with the following changes.

By the way, I still see cppcheck issues in AMF when I run ‘make cppcheck’, but 
this is a great improvement.

Thanks
Gary

diff --git a/src/amf/amfd/csi.cc b/src/amf/amfd/csi.cc
--- a/src/amf/amfd/csi.cc
+++ b/src/amf/amfd/csi.cc
@@ -1399,12 +1399,13 @@ void avd_csi_remove_csiattr(AVD_CSI *csi
 
 void avd_csi_add_csiattr(AVD_CSI *csi, AVD_CSI_ATTR *csiattr)
 {
-   int cnt = 0;
+   int cnt = 1;
AVD_CSI_ATTR *ptr;
 
TRACE_ENTER();
/* Count number of attributes (multivalue) */
ptr = csiattr;
+   osafassert(ptr != nullptr);
while (ptr->attr_next != nullptr) {
cnt++;
ptr = ptr->attr_next;
diff --git a/src/amf/amfnd/cpm.cc b/src/amf/amfnd/cpm.cc
--- a/src/amf/amfnd/cpm.cc
+++ b/src/amf/amfnd/cpm.cc
@@ -145,7 +145,7 @@ void avnd_comp_pm_rec_del(AVND_CB *cb, A
LOG_NO("PM Rec doesn't exist in Comp '%s' of pid %llu", 
comp->name.c_str(), pid);
}
 
-   delete rec; /* rec is no more, dont use it */
+   rec = nullptr;  /* rec is no more, dont use it */
 
/* remove the corresponding element from mon_req list */
rc = avnd_mon_req_del(cb, pid);


-Original Message-
From: A V Mahesh 
Date: Wednesday, 8 March 2017 at 11:28 pm
To: , gary , 
, 
Cc: 
Subject: [PATCH 0 of 3] Review Request for amf: Fix all Cppcheck 1.77 issues 
[#2341] V1

Summary:amf: Fix all Cppcheck 1.77 issues [#2341] V1 
Review request for Trac Ticket(s): #2341
Peer Reviewer(s): Amf Dev
Pull request to: <>
Affected branch(es): default, 5.2
Development branch: default


Impacted area   Impact y/n

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


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

changeset b8e7593cf4eadaa6169fe635ac0df67560ea875a
Author: A V Mahesh 
Date:   Wed, 08 Mar 2017 17:52:21 +0530

amfd: Fix all Cppcheck 1.77 issues [#2341] V1

[staging/src/amf/amfd/app.cc:285]: (style) The scope of the variable 
'i' can
be reduced. [staging/src/amf/amfd/apptype.cc:137]: (style) Condition 
'rc!=0'
is always false [staging/src/amf/amfd/apptype.cc:69]: (style) The scope 
of
the variable 'sg_type' can be reduced. 
[staging/src/amf/amfd/chkop.cc:1297]
-> [staging/src/amf/amfd/chkop.cc:1302]: (style) Variable 'uba' is
reassigned a value before the old one has been used.
[staging/src/amf/amfd/ckpt_dec.cc:374] ->
[staging/src/amf/amfd/ckpt_dec.cc:382]: (style) Variable 'status' is
reassigned a value before the old one has been used.
[staging/src/amf/amfd/ckpt_dec.cc:573] ->
[staging/src/amf/amfd/ckpt_dec.cc:577]: (style) Variable 'status' is
reassigned a value before the old one has been used.
[staging/src/amf/amfd/ckpt_dec.cc:1109]: (performance) Prefer prefix 
++/--
operators for non-primitive types. 
[staging/src/amf/amfd/ckpt_edu.cc:51] ->
[staging/src/amf/amfd/ckpt_edu.cc:56]: (style) Variable 'rc' is 
reassigned a
value before the old one has been used.
[staging/src/amf/amfd/ckpt_enc.cc:2281] ->
[staging/src/amf/amfd/ckpt_enc.cc:2288]: (style) Variable 'status' is
reassigned a value before the old one has been used.
[staging/src/amf/amfd/ckpt_enc.cc:2314] ->
[staging/src/amf/amfd/ckpt_enc.cc:2322]: (style) Variable 'status' is
reassigned a value before the old one has been used.
[staging/src/amf/amfd/ckpt_enc.cc:1951]: (performance) Prefer prefix 
++/--
operators for non-primitive types. 
[staging/src/amf/amfd/ckpt_enc.cc:1982]:
(performance) Prefer prefix ++/-- operators for non-primitive types.
[staging/src/amf/amfd/ckpt_enc.cc:2015]: (performance) Prefer prefix 
++/--
operators for non-primitive types. 
[staging/src/amf/amfd/ckpt_enc.cc:2044]:
(performance) Prefer prefix ++/-- operators for non-primitive types.
[staging/src/amf/amfd/ckpt_enc.cc:2076]: (performance) Prefer prefix 
++/--
operators for non-primitive types. 
[staging/src/amf/amfd/ckpt_enc.cc:2111]:
(performance) Prefer prefix ++/-- operators for non-primitive types.
[staging/src/amf/amfd/ckpt_enc.cc:2151]: (performance)