Ack. Code review only. Regards, Vu
> -----Original Message----- > From: minh chau [mailto:[email protected]] > Sent: Friday, December 16, 2016 8:52 AM > To: praveen malviya <[email protected]>; > [email protected] > Cc: [email protected] > Subject: Re: [PATCH 1 of 1] NTFD: Increment iterator in NotificationMap > loop before deleted [#2223] > > Hi Praveen, > > I have updated the steps, basically it's same as #2219. > > @Vu: Any comments? > > Thanks, > Minh > > On 15/12/16 21:46, praveen malviya wrote: > > Ack, code review only. > > > > Please update the ticket if some easily reproducible steps are there. > > > > Thanks, > > Praveen > > > > On 13-Dec-16 6:01 AM, Minh Hon Chau wrote: > >> 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
