Hi Canh, I have not had time to do any proper review yet but I saw that you have not named the version constant according to Google style guide
Thanks Lennart > -----Original Message----- > From: minh chau [mailto:minh.c...@dektech.com.au] > Sent: den 27 september 2017 06:40 > To: Canh Van Truong <canh.v.tru...@dektech.com.au>; Lennart Lund > <lennart.l...@ericsson.com> > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 1/1] ntf: fix incorrect handling of version when > initializing > OpenSAF APIs in ntf service [#2517] > > Hi Canh, > > ack from me for code review. > > Thanks, > Minh > > On 21/09/17 19:36, Canh Van Truong wrote: > > The version used when initializing the API is in many cases stored in a > > global > > variable and this global variable is used every time the API is initialized. > > The version is given as a pointer to this variable. The problem is that this > > variable is defined as an [in/out] parameter that gives the version to use > when > > initializing [in] but the value shall be changed by the agent to the highest > > version supported by the initialized service [out]. This means that the next > > time the API is initialized it may be initialized with a higher version. > > > > This patch help to fix that always initialize OpenSaf API with correct > > version > in ntf service. > > --- > > src/ntf/ntfd/NtfLogger.cc | 6 ++++-- > > src/ntf/ntfd/ntfs_clm.c | 11 ++++++++--- > > src/ntf/ntfimcnd/ntfimcn_imm.c | 8 +++++--- > > src/ntf/ntfimcnd/ntfimcn_notifier.c | 4 +++- > > src/ntf/tools/ntfclient.c | 4 +++- > > 5 files changed, 23 insertions(+), 10 deletions(-) > > > > diff --git a/src/ntf/ntfd/NtfLogger.cc b/src/ntf/ntfd/NtfLogger.cc > > index 84014b578..d1dc48d18 100644 > > --- a/src/ntf/ntfd/NtfLogger.cc > > +++ b/src/ntf/ntfd/NtfLogger.cc > > @@ -66,9 +66,9 @@ void saLogWriteLogCallback(SaInvocationT invocation, > SaAisErrorT error); > > > > static SaLogCallbacksT logCallbacks = {NULL, NULL, saLogWriteLogCallback}; > > > > -static SaVersionT logVersion = {'A', 2, 1}; > > static SaLogHandleT logHandle; > > static SaLogStreamHandleT alarmStreamHandle; > > +const SaVersionT KLogVersion = {'A', 2, 3}; > > > > NtfLogger::NtfLogger() : readCounter(0) { > > if (SA_AIS_OK != initLog()) { > > @@ -247,18 +247,20 @@ SaAisErrorT NtfLogger::initLog() { > > SaNameT alarmStreamName; > > osaf_extended_name_lend(SA_LOG_STREAM_ALARM, > &alarmStreamName); > > int first_try = 1; > > + SaVersionT log_version = KLogVersion; > > > > TRACE_ENTER(); > > > > /* Initialize the Log service */ > > do { > > - result = saLogInitialize(&logHandle, &logCallbacks, &logVersion); > > + result = saLogInitialize(&logHandle, &logCallbacks, &log_version); > > if (SA_AIS_ERR_TRY_AGAIN == result) { > > if (first_try) { > > LOG_WA("saLogInitialize returns try again, retries..."); > > first_try = 0; > > } > > usleep(AIS_TIMEOUT); > > + log_version = KLogVersion; > > } > > } while (SA_AIS_ERR_TRY_AGAIN == result); > > > > diff --git a/src/ntf/ntfd/ntfs_clm.c b/src/ntf/ntfd/ntfs_clm.c > > index 1f63def8f..cd00fa7b4 100644 > > --- a/src/ntf/ntfd/ntfs_clm.c > > +++ b/src/ntf/ntfd/ntfs_clm.c > > @@ -18,6 +18,9 @@ > > #include "ntf/ntfd/ntfs_com.h" > > #include "base/osaf_time.h" > > > > +const SaVersionT KClmVersion = {'B', 0x04, 0x01}; > > + > > + > > /* > > * @brief CLM callback for tracking node membership status. > > * Depending upon the membership status (joining/leaving > > cluster) > > @@ -99,7 +102,6 @@ done: > > return; > > } > > > > -static SaVersionT clmVersion = {'B', 0x04, 0x01}; > > static const SaClmCallbacksT_4 clm_callbacks = { > > 0, ntfs_clm_track_cbk /*saClmClusterTrackCallback*/ > > }; > > @@ -113,15 +115,18 @@ void *ntfs_clm_init_thread(void *cb) > > { > > ntfs_cb_t *_ntfs_cb = (ntfs_cb_t *)cb; > > SaAisErrorT rc = SA_AIS_OK; > > + SaVersionT clm_version = KClmVersion; > > > > TRACE_ENTER(); > > > > - rc = saClmInitialize_4(&_ntfs_cb->clm_hdl, &clm_callbacks, > &clmVersion); > > + rc = saClmInitialize_4(&_ntfs_cb->clm_hdl, &clm_callbacks, > > + &clm_version); > > while ((rc == SA_AIS_ERR_TRY_AGAIN) || (rc == > SA_AIS_ERR_TIMEOUT) || > > (rc == SA_AIS_ERR_UNAVAILABLE)) { > > osaf_nanosleep(&kHundredMilliseconds); > > + clm_version = KClmVersion; > > rc = saClmInitialize_4(&_ntfs_cb->clm_hdl, &clm_callbacks, > > - &clmVersion); > > + &clm_version); > > } > > if (rc != SA_AIS_OK) { > > LOG_ER("saClmInitialize failed with error: %d", rc); > > diff --git a/src/ntf/ntfimcnd/ntfimcn_imm.c > b/src/ntf/ntfimcnd/ntfimcn_imm.c > > index de96e3020..a1daf0e16 100644 > > --- a/src/ntf/ntfimcnd/ntfimcn_imm.c > > +++ b/src/ntf/ntfimcnd/ntfimcn_imm.c > > @@ -50,7 +50,7 @@ extern ntfimcn_cb_t ntfimcn_cb; /* See > ntfimcn_main.c */ > > * Global, scope file > > */ > > /* Release code, major version, minor version */ > > -#define IMM_VERSION {'A', 2, 12} > > +static const SaVersionT KImmVersion = {'A', 2, 12}; > > static const uint64_t sleep_delay_ms = 500; > > static const uint64_t max_waiting_time_7s = (7 * 1000); /* 7 sec */ > > static const uint64_t max_waiting_time_60s = (60 * 1000); /* 60 sec */ > > @@ -784,7 +784,7 @@ int ntfimcn_imm_init(ntfimcn_cb_t *cb) > > goto done; > > } > > cb->immOiHandle = 0; > > - SaVersionT imm_version = IMM_VERSION; > > + SaVersionT imm_version = KImmVersion; > > rc = saImmOiInitialize_2(&cb->immOiHandle, &callbacks, > > &imm_version); > > while ( > > @@ -805,6 +805,7 @@ int ntfimcn_imm_init(ntfimcn_cb_t *cb) > > } > > } > > cb->immOiHandle = 0; > > + imm_version = KImmVersion; > > rc = saImmOiInitialize_2(&cb->immOiHandle, > &callbacks, > > &imm_version); > > } > > @@ -927,7 +928,7 @@ static bool initializeImmOmHandle(SaImmHandleT* > immOmHandle) { > > struct timespec delay_ts; > > SaAisErrorT ais_rc; > > bool internal_rc = true; > > - SaVersionT imm_version = IMM_VERSION; > > + SaVersionT imm_version = KImmVersion; > > > > osaf_millis_to_timespec(sleep_delay_ms, &delay_ts); > > osaf_set_millis_timeout(max_waiting_time_60s, &timeout_ts); > > @@ -939,6 +940,7 @@ static bool initializeImmOmHandle(SaImmHandleT* > immOmHandle) { > > break; > > } > > osaf_nanosleep(&delay_ts); > > + imm_version = KImmVersion; > > } > > > > if (ais_rc != SA_AIS_OK) { > > diff --git a/src/ntf/ntfimcnd/ntfimcn_notifier.c > b/src/ntf/ntfimcnd/ntfimcn_notifier.c > > index 9aba87cb6..5db059722 100644 > > --- a/src/ntf/ntfimcnd/ntfimcn_notifier.c > > +++ b/src/ntf/ntfimcnd/ntfimcn_notifier.c > > @@ -39,7 +39,7 @@ > > #include "ntfimcn_main.h" > > > > /* Release code, major version, minor version */ > > -SaVersionT ntf_version = {'A', 0x01, 0x01}; > > +const SaVersionT KNtfVersion = {'A', 0x01, 0x01}; > > const unsigned int sleep_delay_ms = 500; > > const unsigned int max_waiting_time_ms = 7 * 1000; /* 7 secs */ > > const unsigned int max_init_waiting_time_ms = 60 * 1000; /* 60 secs */ > > @@ -60,6 +60,7 @@ int ntfimcn_ntf_init(ntfimcn_cb_t *cb) > > { > > SaAisErrorT rc; > > int internal_rc = 0; > > + SaVersionT ntf_version = KNtfVersion; > > > > TRACE_ENTER(); > > > > @@ -86,6 +87,7 @@ int ntfimcn_ntf_init(ntfimcn_cb_t *cb) > > } > > } > > cb->ntf_handle = 0; > > + ntf_version = KNtfVersion; > > rc = saNtfInitialize(&cb->ntf_handle, NULL, > > &ntf_version); > > } > > diff --git a/src/ntf/tools/ntfclient.c b/src/ntf/tools/ntfclient.c > > index dcc957b7f..472be4bc3 100644 > > --- a/src/ntf/tools/ntfclient.c > > +++ b/src/ntf/tools/ntfclient.c > > @@ -1099,11 +1099,13 @@ SaAisErrorT > ntftool_saNtfInitialize(SaNtfHandleT *ntfHandle, > > { > > int curRetryTime = 0; > > SaAisErrorT rc; > > + SaVersionT ntf_version = *version; > > do { > > - rc = saNtfInitialize(ntfHandle, ntfCallbacks, version); > > + rc = saNtfInitialize(ntfHandle, ntfCallbacks, &ntf_version); > > if (rc == SA_AIS_ERR_TRY_AGAIN) { > > sleep(gl_apiRetry); > > curRetryTime += gl_apiRetry; > > + ntf_version = *version; > > } else > > break; > > } while (curRetryTime < gl_apiTolerance); ------------------------------------------------------------------------------ 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 Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
