Dear Mahesh, The first call erase a element with id from map. The second call with same id, no element found so iterator will point to map->end(). Because of that map->erase() does not execute.
I am sorry that I cannot find the ER that you point out. Sincerely, Hoang -----Original Message----- From: A V Mahesh [mailto:mahesh.va...@oracle.com] Sent: Friday, November 25, 2016 1:20 PM To: Vo Minh Hoang <hoang.m...@dektech.com.au> Cc: anders.wid...@ericsson.com; opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1 of 1] cpnd: fix error handling while section_hdr_update_fail [#2207] Hi Hoang, The second invocation of cpnd_ckpt_sec_del_db() with generate code dump. ============================================================= SectionMap *map(static_cast<SectionMap *>(cp_node->replica_info.section_db)); if (map) { SectionMap::iterator it(map->find(id)); if (it != map->end()) { sectionInfo = it->second; map->erase(it); } } else { LOG_ER("can't find map in cpnd_ckpt_sec_del"); *osafassert(false);* } ============================================================= -AVM On 11/25/2016 11:16 AM, Vo Minh Hoang wrote: > Dear Mahesh, > > Thank you very much for your comment. > > Would you please verify again my understanding about this. > > Old cpnd_ckpt_sec_del() function and new cpnd_ckpt_sec_del_db() search > for the sectionInfo in 2 maps by its id and remove if found. > In case ckpt_hdr_update_fails, the cpnd_ckpt_sec_del_db() is called > twice, the first one remove sectionInfo from maps, the second one does > not found sectionInfo and cannot remove anything, will not generate error. > > Sincerely, > Hoang > > -----Original Message----- > From: A V Mahesh [mailto:mahesh.va...@oracle.com] > Sent: Friday, November 25, 2016 12:12 PM > To: Hoang Vo <hoang.m...@dektech.com.au>; anders.wid...@ericsson.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 1 of 1] cpnd: fix error handling while > section_hdr_update_fail [#2207] > > Hi Hoang, > > NACK > > This patch has some fundamental problem, this will corrupt the cpsv > database . > This patch instead of resolving the issue , increase the percentage of > problem occurrence . > > Basically this patch splits cpnd_ckpt_sec_del() in to two parts > cpnd_ckpt_sec_del_db() & cpnd_ckpt_sec_del() and the new > cpnd_ckpt_sec_del() invokes cpnd_ckpt_sec_del_db() to fullfil the old > function behavior , so that to support other old invocations and > changed the error cleanup in > cpnd_ckpt_sec_add() as follows : > > ckpt_hdr_update_fails: > cpnd_ckpt_sec_del(cp_node, id); > > section_hdr_update_fails: > cpnd_ckpt_sec_del_db(cp_node, id); > > This means cpnd_ckpt_sec_del_db() is always called twice in case of > ckpt_hdr_update_fails , which will generate core > > -AVM > > > On 11/24/2016 3:26 PM, Hoang Vo wrote: >> osaf/libs/common/cpsv/include/cpnd_sec.h | 3 +++ >> osaf/services/saf/cpsv/cpnd/cpnd_db.c | 4 +++- >> osaf/services/saf/cpsv/cpnd/cpnd_sec.cc | 31 > +++++++++++++++++++++++++++---- >> 3 files changed, 33 insertions(+), 5 deletions(-) >> >> >> problem: >> the steps to add a section is add_db_tree -> update_sec_hdr -> >> update_ckpt_hdr so if an error occur cpsv should handle error in >> reverse > order. >> currently, section_hdr_update_fails, cpsv revert ckpt_hdr also that >> case error >> >> solution: >> only revert db_tree in case section_hdr_update_fails >> >> diff --git a/osaf/libs/common/cpsv/include/cpnd_sec.h >> b/osaf/libs/common/cpsv/include/cpnd_sec.h >> --- a/osaf/libs/common/cpsv/include/cpnd_sec.h >> +++ b/osaf/libs/common/cpsv/include/cpnd_sec.h >> @@ -39,6 +39,9 @@ CPND_CKPT_SECTION_INFO * >> cpnd_ckpt_sec_get_create(const CPND_CKPT_NODE *, const >> SaCkptSectionIdT *); >> >> CPND_CKPT_SECTION_INFO * >> +cpnd_ckpt_sec_del_db(CPND_CKPT_NODE *, SaCkptSectionIdT *); >> + >> +CPND_CKPT_SECTION_INFO * >> cpnd_ckpt_sec_del(CPND_CKPT_NODE *, SaCkptSectionIdT *); >> >> CPND_CKPT_SECTION_INFO * >> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_db.c >> b/osaf/services/saf/cpsv/cpnd/cpnd_db.c >> --- a/osaf/services/saf/cpsv/cpnd/cpnd_db.c >> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_db.c >> @@ -468,10 +468,12 @@ CPND_CKPT_SECTION_INFO *cpnd_ckpt_sec_ad >> TRACE_LEAVE(); >> return pSecPtr; >> >> - section_hdr_update_fails: >> ckpt_hdr_update_fails: >> cpnd_ckpt_sec_del(cp_node, id); >> >> + section_hdr_update_fails: >> + cpnd_ckpt_sec_del_db(cp_node, id); >> + >> section_add_fails: >> if (pSecPtr->sec_id.id != NULL) >> m_MMGR_FREE_CPND_DEFAULT(pSecPtr->sec_id.id); >> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc >> b/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc >> --- a/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc >> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc >> @@ -157,19 +157,19 @@ cpnd_ckpt_sec_find(const CPND_CKPT_NODE >> } >> >> > /********************************************************************* > ****** > * >> - * Name : cpnd_ckpt_sec_del >> + * Name : cpnd_ckpt_sec_del_db >> * >> - * Description : Function to remove the section from a checkpoint. >> + * Description : Function to remove the section from a checkpoint map > db. >> * >> * Arguments : CPND_CKPT_NODE *cp_node - Check point node. >> * : SaCkptSectionIdT id - Section Identifier >> - * >> + * >> * Return Values : ptr to CPND_CKPT_SECTION_INFO/NULL; >> * >> * Notes : None. >> > ********************************************************************** > ****** > */ >> CPND_CKPT_SECTION_INFO * >> -cpnd_ckpt_sec_del(CPND_CKPT_NODE *cp_node, SaCkptSectionIdT *id) >> +cpnd_ckpt_sec_del_db(CPND_CKPT_NODE *cp_node, SaCkptSectionIdT *id) >> { >> CPND_CKPT_SECTION_INFO *sectionInfo(0); >> >> @@ -206,6 +206,29 @@ cpnd_ckpt_sec_del(CPND_CKPT_NODE *cp_nod >> osafassert(false); >> } >> >> + TRACE_LEAVE(); >> + >> + return sectionInfo; >> +} >> + >> > +/******************************************************************** > +****** > ** >> + * Name : cpnd_ckpt_sec_del >> + * >> + * Description : Function to remove the section from a checkpoint. >> + * >> + * Arguments : CPND_CKPT_NODE *cp_node - Check point node. >> + * : SaCkptSectionIdT id - Section Identifier >> + * >> + * Return Values : ptr to CPND_CKPT_SECTION_INFO/NULL; >> + * >> + * Notes : None. >> + >> +******************************************************************** >> +* >> +********/ >> +CPND_CKPT_SECTION_INFO * >> +cpnd_ckpt_sec_del(CPND_CKPT_NODE *cp_node, SaCkptSectionIdT *id) { >> + TRACE_ENTER(); >> + CPND_CKPT_SECTION_INFO *sectionInfo = >> +cpnd_ckpt_sec_del_db(cp_node, id); >> + >> if (sectionInfo) { >> cp_node->replica_info.n_secs--; >> cp_node->replica_info.mem_used = >> cp_node->replica_info.mem_used >> - (sectionInfo->sec_size); > ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
