Hi zoran, comments inline.
Thanks, Neel. On Monday 18 January 2016 06:46 PM, Zoran Milinkovic wrote: > Hi Neelakanta, > > If you only change osafassert, then the similar osafassert must be added in > immnd_evt_proc_ccb_finalize() after immnd_evt_ccb_abort(). > And another check must be added in "do-while" loop in > immnd_evt_proc_ccb_finalize() for checking that clientArr[ix] is not 0. This is not required, as clientArr will be accessed only if clArrSize is more than 1 in immnd_evt_proc_ccb_finalize. > I feel more safely if client connections with 0 are removed from client array. > > Best regards, > Zoran > > > -----Original Message----- > From: Neelakanta Reddy [mailto:[email protected]] > Sent: Monday, January 18, 2016 12:37 PM > To: Zoran Milinkovic; Hung Duc Nguyen > Cc: [email protected] > Subject: Re: [PATCH 1 of 1] imm: corrected the assert checking in CCb apply > when CCB is aborted [#1664] > > Hi zoran, > > yes, clArr should be freed here also. I added in finalize, but forgot to add > here. > Re publish the patch again. > comments inline. > > Thanks, > Neel. > On Monday 18 January 2016 04:47 PM, Zoran Milinkovic wrote: >> Hi Neelakanta, >> >> Memory of clArr is not freed. It was one of comments in the review of #1503. >> >> If client connection is 0, then it should be removed from clArr (clArrSize >> will be decreased), and osafassert can remain as it was before the patch. >> The change can be done in immModel_ccbAbort() removing client connections of >> 0. >> >> In the patch, clArr can still point to NULL if immModel_ccbAbort() in >> immnd_evt_ccb_abort() returns true. Then osafassert will crash in try >> to access clArr[0]; > The below modified assert can resolve when the clArr NULL. > osafassert(!clArrSize||!clArr[0] || originatedAtThisNd); >> Nack from me. >> >> Best regards, >> Zoran >> >> >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] >> Sent: Monday, January 18, 2016 7:28 AM >> To: Zoran Milinkovic; Hung Duc Nguyen >> Cc: [email protected] >> Subject: [PATCH 1 of 1] imm: corrected the assert checking in CCb >> apply when CCB is aborted [#1664] >> >> osaf/services/saf/immsv/immnd/immnd_evt.c | 6 +++++- >> 1 files changed, 5 insertions(+), 1 deletions(-) >> >> >> diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c >> b/osaf/services/saf/immsv/immnd/immnd_evt.c >> --- a/osaf/services/saf/immsv/immnd/immnd_evt.c >> +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c >> @@ -7899,7 +7899,11 @@ static void immnd_evt_proc_ccb_apply(IMM >> /*err != SA_AIS_OK => generate SaImmOiCcbAbortCallbackT >> upcalls >> */ >> immnd_evt_ccb_abort(cb, evt->info.ccbId, &clArr, >> &clArrSize, NULL); >> - osafassert(!clArrSize || originatedAtThisNd); >> + /* when the client is not originated from this ND then >> the client >> + connection must be zero. We are in apply and there >> will be only >> + one implementer connection and no augumentaion >> connections. >> + */ >> + osafassert(!clArr[0] || originatedAtThisNd); >> } >> TRACE_2("CCB APPLY TERMINATING CCB: %u", evt->info.ccbId); >> bCcbFinalize = 1; ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
