Hi Minh, Please ignore my comments. Thanks.
Regards, Vu > -----Original Message----- > From: Vu Minh Nguyen [mailto:[email protected]] > Sent: Tuesday, December 13, 2016 5:32 PM > To: 'Minh Hon Chau' <[email protected]>; > '[email protected]' <[email protected]> > Cc: '[email protected]' <opensaf- > [email protected]> > Subject: RE: [PATCH 1 of 1] NTFD: Increment iterator in NotificationMap > loop before deleted [#2223] > > Hi Minh, > > I have made an sample code (http://ideone.com/yL1wjx) that do basically > same as this below code, > and I see 02 points: > > for (posNot = notificationMap.begin(); > posNot != notificationMap.end();) { > NotificationMap::iterator deleteNot = posNot++; > > 1) If the last element is erased, the loop will never come to an end (infinitive > loop) > 2) If erasing a other element, not the last, then the next element will be > stepped over (not be checked) > > Seems, when an element is erased, all elements are reallocated. > In other words, do increment and erasing will make the iterator step forward > twice. > > Other thing, I am a bit confused about the coredump and the fix. > I see the last back trace located at the function `getNextSubscription`. > If `posNot` is out of range, then it probably get coredump at the time of > accessing to `posNot` data? > > Regards, Vu > > > -----Original Message----- > > From: Minh Hon Chau [mailto:[email protected]] > > Sent: Tuesday, December 13, 2016 7:31 AM > > To: [email protected]; [email protected]; > > [email protected] > > Cc: [email protected] > > Subject: [PATCH 1 of 1] NTFD: Increment iterator in NotificationMap loop > > before deleted [#2223] > > > > osaf/services/saf/ntfsv/ntfs/NtfAdmin.cc | 7 ++++--- > > 1 files changed, 4 insertions(+), 3 deletions(-) > > > > > > In NtfAdmin::checkNotificationList(), when deleteConfirmedNotification() > > deletes > > the last item, the NotificationMap is reduced, posNot++ will be out of > range > > of > > NotificationMap since posNot is invalid. > > > > Patch uses posNot is main iterator, and deleteNot as temporary iterator > > points to > > deleted notification > > > > diff --git a/osaf/services/saf/ntfsv/ntfs/NtfAdmin.cc > > b/osaf/services/saf/ntfsv/ntfs/NtfAdmin.cc > > --- a/osaf/services/saf/ntfsv/ntfs/NtfAdmin.cc > > +++ b/osaf/services/saf/ntfsv/ntfs/NtfAdmin.cc > > @@ -646,9 +646,10 @@ void NtfAdmin::checkNotificationList() { > > TRACE_ENTER(); > > NotificationMap::iterator posNot; > > for (posNot = notificationMap.begin(); > > - posNot != notificationMap.end(); > > - posNot++) { > > + posNot != notificationMap.end();) { > > NtfSmartPtr notification = posNot->second; > > + NotificationMap::iterator deleteNot = posNot++; > > + > > if (notification->loggedOk() == false) { > > /* When reader API works check if already logged */ > > logger.log(notification, true); > > @@ -668,7 +669,7 @@ void NtfAdmin::checkNotificationList() { > > TRACE_2("Client: %u not exist", uSubId.clientId); > > } > > } > > - deleteConfirmedNotification(notification, posNot); > > + deleteConfirmedNotification(notification, deleteNot); > > } > > } > > 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 [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
