Hi Neelakanta, Thanks for the info. You can push the patch with fixing earlier comments. No need to publish the patch again.
Ack from me. Best regards, Zoran -----Original Message----- From: Neelakanta Reddy [mailto:[email protected]] Sent: Tuesday, January 19, 2016 6:00 AM 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, comments inline. On Monday 18 January 2016 07:43 PM, Zoran Milinkovic wrote: > Hi Neelakanta, > > If clArrSize is not 0 and clientArr array contains client connection 0, then > we shall start getting warnings in syslogs that a client is down, and that is > wrong. The above syslog warning will not happen, because The originatedAtThisNd must be true. originatedAtThisNd will be true with the following scenarios only: 1. OmFinalize is called the originatedAtThisNd will be true. 2. if the nodeid is matching the client connection nodeid then only the originatedAtThisNd will be true for the aborted ccb . But, the case explained by you may not occur because if client connection is 0, means it is not originated from this ND . Thanks, Neel. > Best regards, > Zoran > > -----Original Message----- > From: Neelakanta Reddy [mailto:[email protected]] > Sent: Monday, January 18, 2016 1:35 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, > > 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
