Hi Gary, fine, I'll change the patch to your suggestion.
/Thanks -----Original Message----- From: Hans Nordebäck [mailto:[email protected]] Sent: den 30 oktober 2017 13:00 To: Gary Lee <[email protected]> Cc: Minh Hon Chau <[email protected]>; [email protected] Subject: Re: [devel] [PATCH 1/1] amfnd: fix segv in ncs_tmr_stop [#2658] Hi Gary, yes, if the iter is not used again it should work/Thanks HansN On 10/30/2017 12:56 PM, Gary Lee wrote: > Hi Hans > > I guess the crashes occured because an invalidated iter was used after > avnd_last_step_clean. If we removed the element from dnd_list before > calling avnd_last_step_clean, and thus not use iter again, it should > be ok? > > Thanks > Gary > > On 30 Oct. 2017 22:42, Hans Nordebäck <[email protected]> wrote: > > Hi Gary, > > the problem is that the dnd_list is global and during iteration it is > changed elsewhere > > via funciton avnd_last_step_clean(). I guess if comparing dnd_list > and > tmp after the call they may > > differ in some way. Perhaps tmp can be updated with these changes > or we > have to > > refactor this code. > > Thanks HansN > > > On 10/30/2017 03:40 AM, Gary Lee wrote: > > Hi Hans > > > > Ack (review only). Would this also do the job? > > > > diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc > > index 61cea97..d678bd4 100644 > > --- a/src/amf/amfnd/di.cc > > +++ b/src/amf/amfnd/di.cc > > @@ -1285,13 +1285,15 @@ void avnd_di_msg_ack_process(AVND_CB > *cb, uint32_t mid) { > > > > // matching record > > if (msg_id == mid) { > > + cb->dnd_list.erase(iter); > > + // iter is now invalid, exit iterator loop asap > > + > > if (rec->msg.info.avd->msg_type == > AVSV_N2D_NODE_DOWN_MSG) { > > // first to stop timer to avoid processing timeout event > > // then perform last step clean up > > avnd_stop_tmr(cb, &rec->resp_tmr); > > avnd_last_step_clean(cb); > > } > > - cb->dnd_list.erase(iter); > > TRACE("remove msg %u from queue", msg_id); > > avnd_diq_rec_del(cb, rec); > > break; > > > > > > > > On 28/10/17, 2:40 am, "Hans Nordeback" > <[email protected]> wrote: > > > > --- > > src/amf/amfnd/di.cc | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc > > index 7aac34260..6e5aaf54b 100644 > > --- a/src/amf/amfnd/di.cc > > +++ b/src/amf/amfnd/di.cc > > @@ -1293,7 +1293,9 @@ uint32_t > avnd_di_node_down_msg_send(AVND_CB *cb) > > void avnd_di_msg_ack_process(AVND_CB *cb, uint32_t mid) { > > TRACE_ENTER2("%u", mid); > > > > - for (auto iter = cb->dnd_list.begin(); iter != > cb->dnd_list.end(); ++iter) { > > + std::vector<AVND_DND_MSG_LIST*> tmp = cb->dnd_list; > > + > > + for (auto iter = tmp.begin(); iter != tmp.end(); ++iter) { > > auto rec = *iter; > > osafassert(rec->msg.type == AVND_MSG_AVD); > > const uint32_t msg_id = > *(reinterpret_cast<uint32_t*>(&(rec->msg.info.avd->msg_info))); > > @@ -1306,12 +1308,13 @@ void > avnd_di_msg_ack_process(AVND_CB *cb, uint32_t mid) { > > avnd_stop_tmr(cb, &rec->resp_tmr); > > avnd_last_step_clean(cb); > > } > > - cb->dnd_list.erase(iter); > > + tmp.erase(iter); > > TRACE("remove msg %u from queue", msg_id); > > avnd_diq_rec_del(cb, rec); > > break; > > } > > } > > + cb->dnd_list = tmp; > > > > TRACE_LEAVE2(); > > return; > > -- > > 2.14.2 > > > > > > > > > > ------------------------------------------------------------------------------ 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 ------------------------------------------------------------------------------ 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
