Hi srinivas,
Please look at my comments with tag [Canh] Regards Canh -----Original Message----- From: srinivas [mailto:[email protected]] Sent: Tuesday, March 20, 2018 4:21 PM To: [email protected]; [email protected] Cc: [email protected] Subject: [devel] [PATCH 1/1] ntf: fix to avoid core dump of osafntfimcnd processs [#2806] --- src/ntf/ntfimcnd/ntfimcn_imm.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/ntf/ntfimcnd/ntfimcn_imm.c b/src/ntf/ntfimcnd/ntfimcn_imm.c index 1358d9f..efa8b1e 100644 --- a/src/ntf/ntfimcnd/ntfimcn_imm.c +++ b/src/ntf/ntfimcnd/ntfimcn_imm.c @@ -152,7 +152,6 @@ static SaAisErrorT getClassDescription(const SaImmClassNameT className, * * Note: * Uses in file global struct s_get_rdn_attr_name - * Uses global immOmHandle in struct ntfimcn_cb_t * * @param className[in] * @@ -231,19 +230,15 @@ error: /* NOTE: Resources are allocated by this function * saImmOmClassDescriptionMemoryFree_2() must be called before * returning from this function. Not done here because of - * osafassert() + * imcn_exit() */ LOG_ER("%s Failed", __FUNCTION__); if ( rc == SA_AIS_ERR_NOT_EXIST ) { - LOG_NO("osafntfimcnd restarting due to error %s.", - saf_error(rc)); - _exit(EXIT_FAILURE); - } - else - { - // core dump will be generated for all other errors which will be used - // for further analysis of the error. - osafassert(0); + LOG_NO("%s: Failed with error %s", __FUNCTION__,saf_error(rc)); + imcn_exit(EXIT_FAILURE); + } else { + LOG_NO("osafntfimcnd restarting due to an error."); + imcn_exit(EXIT_FAILURE); [Canh] Why if ... else... here? It seems that both of them process the same meaning here. } return 0; /* Dummy */ } @@ -936,7 +931,7 @@ done: static bool initializeImmOmHandle(SaImmHandleT* immOmHandle) { struct timespec timeout_ts; struct timespec delay_ts; - SaAisErrorT ais_rc; + SaAisErrorT ais_rc = SA_AIS_OK; bool internal_rc = true; SaVersionT imm_version = kImmVersion; @@ -946,11 +941,17 @@ static bool initializeImmOmHandle(SaImmHandleT* immOmHandle) { while (osaf_is_timeout(&timeout_ts) == false) { ais_rc = saImmOmInitialize(immOmHandle, &omCallbacks, &imm_version); - if (ais_rc != SA_AIS_ERR_TRY_AGAIN) { + if (ais_rc == SA_AIS_ERR_TRY_AGAIN) { + osaf_nanosleep(&delay_ts); + imm_version = kImmVersion; + continue; + } else if (ais_rc == SA_AIS_ERR_TIMEOUT && immOmHandle == 0) { [Canh] 1/ Don't compare "immOmHandle == 0". It is output value and of course it is invalid ptr when TIMEOUT return. And the finalizeImmOmHandle() should be placed after osaf_nanosleep(). 2/ Another comment is that I'm not sure if we do try again with TIMEOUT error is right here because in some case we will initialize double om handle. For example following steps: * saImmOmInitialize successful but get TIMEOUT error. (see imm AIS document 4.3.1 saImmOmInitialize() for more clear) * finalizeImmOmHandle unsuccessful * saImmOmInitialize successful without error + finalizeImmOmHandle(*immOmHandle); + osaf_nanosleep(&delay_ts); + imm_version = kImmVersion; + continue; + } else break; - } - osaf_nanosleep(&delay_ts); - imm_version = kImmVersion; } if (ais_rc != SA_AIS_OK) { -- 2.7.4 ---------------------------------------------------------------------------- -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! <http://sdm.link/slashdot> http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list <mailto:[email protected]> [email protected] <https://lists.sourceforge.net/lists/listinfo/opensaf-devel> 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
