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

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

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

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

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 +++

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

2017-03-10 Thread Gary Lee
Hi Mahesh I’m still going through it. Seems to be mainly asserts and coredumps. Eg. csi->num_attributes is not set properly. The fix below seems to work. 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

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

2017-03-10 Thread A V Mahesh
Hi Gary, One of the possibility of patch breaking the legacy Application is following change, try to rollback this change and see: === diff --git a/src/amf/agent/ava_op.cc b/src/amf/agent/ava_op.cc ---

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

2017-03-09 Thread A V Mahesh
Hi Gary, Thanks for the initial inputs. The patch doesn't contain any in-service /backward compatible change , just wondering how this patch broken the legacy Application , can you please code-snippet of your legacy Application, that breaking the patch. -AVM On 3/10/2017 10:59 AM, Gary Lee

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

2017-03-09 Thread Gary Lee
Hi Mahesh Some of our legacy tests are failing after applying this series. I will do some debugging and send back details. Thanks Gary -Original Message- From: A V Mahesh Date: Wednesday, 8 March 2017 at 11:28 pm To: , gary

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

2017-03-08 Thread A V Mahesh
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