ran cppcheck on this patch it added two comments.

otherwise ACK from me.

On 06/14/2016 01:26 PM, reddy.neelaka...@oracle.com wrote:
>   osaf/services/saf/smfsv/smfd/smfd.h              |   4 +
>   osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc |  19 ++++-
>   osaf/services/saf/smfsv/smfd/smfd_cb.h           |   1 +
>   osaf/services/saf/smfsv/smfd/smfd_main.c         |  86 
> ++++++++++++++++++++++-
>   4 files changed, 104 insertions(+), 6 deletions(-)
>
>
> mutex is introduced, when accessing IMM OI handle, between campaign thread 
> and main thread
>
> diff --git a/osaf/services/saf/smfsv/smfd/smfd.h 
> b/osaf/services/saf/smfsv/smfd/smfd.h
> --- a/osaf/services/saf/smfsv/smfd/smfd.h
> +++ b/osaf/services/saf/smfsv/smfd/smfd.h
> @@ -98,6 +98,10 @@ extern "C" {
>   
>       void smfd_cb_lock();
>       void smfd_cb_unlock();
> +     void smfd_imm_lock();
> +     void smfd_imm_unlock();
> +     int smfd_imm_trylock();
> +
>   
>   #ifdef __cplusplus
>   }
> diff --git a/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc 
> b/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc
> --- a/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc
> +++ b/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc
> @@ -743,11 +743,26 @@ uint32_t create_campaign_objects(smfd_cb
>    */
>   uint32_t updateImmAttr(const char *dn, SaImmAttrNameT attributeName, 
> SaImmValueTypeT attrValueType, void *value)
>   {
> +     smfd_imm_lock();
>       SaAisErrorT rc = immutil_update_one_rattr(smfd_cb->campaignOiHandle, 
> dn, attributeName, attrValueType, value);
> +     smfd_imm_unlock();
> +
>       if (rc != SA_AIS_OK) {
> -             LOG_ER("updateImmAttr(): immutil_update_one_rattr FAILED, rc = 
> %d, going to assert", (int)rc);
> -             osafassert(0);
> +             if (rc == SA_AIS_ERR_BAD_HANDLE){
> +                     /* If there is IMMND restart in the middle of campign, 
> there is chance that
> +                      *  the camiaign thread with Rt-update will get 
> timeout, because the update
> +                      *  has been sent to IMMND, before the reply is sent 
> back to OI(SMFD) the IMMND
> +                      *  is restarted and BAD_HANDLE is returned after API 
> timout.
> +                      */
> +
> +                     LOG_WA("updateImmAttr(): immutil_update_one_rattr 
> FAILED, rc = %d,"
> +                                     "OI handle will be resurrected", 
> (int)rc);
> +             } else {
> +                     LOG_ER("updateImmAttr(): immutil_update_one_rattr 
> FAILED, rc = %d, going to assert", (int)rc);
> +                     osafassert(0);
> +             }
>       }
> +
>       return NCSCC_RC_SUCCESS;
>   }
>   
> diff --git a/osaf/services/saf/smfsv/smfd/smfd_cb.h 
> b/osaf/services/saf/smfsv/smfd/smfd_cb.h
> --- a/osaf/services/saf/smfsv/smfd/smfd_cb.h
> +++ b/osaf/services/saf/smfsv/smfd/smfd_cb.h
> @@ -68,6 +68,7 @@ typedef struct smfd_cb {
>       SMFD_SMFND_ADEST_INVID_MAP *smfnd_list;     /* SMFNDs need to respond 
> to the callback. */
>       uint32_t no_of_smfnd;
>       pthread_mutex_t lock;                     /* Used by smfd_cb_t 
> lock/unlock functions */
> +        pthread_mutex_t imm_lock;                 /* Used when IMM OI handle 
> is shared between campaign thread and main thread*/
>       uint32_t maxDnLength;                     /* Max DN length */
>           uint32_t procExecutionMode;               /* Control the procedure 
> execution modes
>                                                        SMF_STANDARD 0
> diff --git a/osaf/services/saf/smfsv/smfd/smfd_main.c 
> b/osaf/services/saf/smfsv/smfd/smfd_main.c
> --- a/osaf/services/saf/smfsv/smfd/smfd_main.c
> +++ b/osaf/services/saf/smfsv/smfd/smfd_main.c
> @@ -96,6 +96,50 @@ void smfd_cb_unlock()
>       }
>   }
>   
> +int smfd_imm_trylock()
> +{
> +     TRACE_ENTER();
> +     int err ;
> +
> +     err = pthread_mutex_trylock(&(smfd_cb->imm_lock));
> +     if (err != 0) {
> +             if ( err == EBUSY ){
> +                     LOG_WA("Lock failed eith EBUSY pthread_mutex_trylock 
> for imm %d", err);
> +                     return 1;
> +             } else {
> +                     LOG_ER("Lock failed pthread_mutex_trylock for imm %d", 
> err);
> +                     abort();
> +             }
> +     }
> +     return 0;
[rafael] this return is never reached
> +     TRACE_LEAVE();
> +}
> +
> +void smfd_imm_lock()
> +{
> +     TRACE_ENTER();
> +     int errno;
[rafael] errno is never used
> +
> +     if (pthread_mutex_lock(&(smfd_cb->imm_lock)) != 0) {
> +             LOG_ER("Lock failed pthread_mutex_lock for imm");
> +             abort();
> +     }
> +     TRACE_LEAVE();
> +}
> +
> +
> +void smfd_imm_unlock()
> +{
> +     TRACE_ENTER();
> +     if (pthread_mutex_unlock(&(smfd_cb->imm_lock)) != 0) {
> +             LOG_ER("Unlock failed pthread_mutex_unlock for imm");
> +             abort();
> +     }
> +     TRACE_LEAVE();
> +}
> +
> +
> +
>   
> /****************************************************************************
>    * Name          : smfd_cb_init
>    *
> @@ -112,7 +156,7 @@ uint32_t smfd_cb_init(smfd_cb_t * smfd_c
>   {
>       TRACE_ENTER();
>   
> -     pthread_mutexattr_t mutex_attr;
> +     pthread_mutexattr_t mutex_attr, mutex_attr1;
>   
>       smfd_cb->amfSelectionObject = -1;
>       smfd_cb->campaignSelectionObject = -1;
> @@ -159,6 +203,28 @@ uint32_t smfd_cb_init(smfd_cb_t * smfd_c
>               LOG_ER("Failed pthread_mutexattr_destroy");
>               return NCSCC_RC_FAILURE;
>       }
> +     if (pthread_mutexattr_init(&mutex_attr1) != 0) {
> +             LOG_ER("Failed pthread_mutexattr_init used for imm");
> +             return NCSCC_RC_FAILURE;
> +     }
> +
> +     if (pthread_mutexattr_settype(&mutex_attr1, PTHREAD_MUTEX_RECURSIVE) != 
> 0) {
> +             LOG_ER("Failed pthread_mutexattr_settype used for imm");
> +             pthread_mutexattr_destroy(&mutex_attr1);
> +             return NCSCC_RC_FAILURE;
> +     }
> +
> +     if (pthread_mutex_init(&(smfd_cb->imm_lock), &mutex_attr1) != 0) {
> +             LOG_ER("Failed pthread_mutex_init used for imm");
> +             pthread_mutexattr_destroy(&mutex_attr1);
> +             return NCSCC_RC_FAILURE;
> +     }
> +
> +     if (pthread_mutexattr_destroy(&mutex_attr1) != 0) {
> +             LOG_ER("Failed pthread_mutexattr_destroy used for imm");
> +             return NCSCC_RC_FAILURE;
> +     }
> +
>   
>       TRACE_LEAVE();
>       return NCSCC_RC_SUCCESS;
> @@ -335,9 +401,21 @@ static void main_process(void)
>   
>               /* Process all the Imm callback events */
>               if (fds[SMFD_COI_FD].revents & POLLIN) {
> -                     if ((error =
> -                          saImmOiDispatch(smfd_cb->campaignOiHandle,
> -                                          SA_DISPATCH_ALL)) != SA_AIS_OK) {
> +                     if ( smfd_imm_trylock() == 0 ) {
> +
> +                             /* In the case of IMMND restart, If trylock 
> succedes then the oiDispatch
> +                              * is called and IMM OI will get resurrected. 
> Otherwise, the campign
> +                              * thread has taken the imm_lock. Campign 
> thread will resurrect the IMM
> +                              * OI handle. trylock is used to unlock the 
> main thread, if the
> +                              * immlock is taken by campaign thread.
> +                              */
> +
> +                             error = 
> saImmOiDispatch(smfd_cb->campaignOiHandle,
> +                                             SA_DISPATCH_ALL);
> +                             smfd_imm_unlock();
> +                     }
> +
> +                     if (error != SA_AIS_OK) {
>                               /*
>                                ** BAD_HANDLE is interpreted as an IMM service 
> restart. Try
>                                ** reinitialize the IMM OI API in a background 
> thread and let


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports. http://pubads.g.doubleclick.net/gampad/clk?id=1444514421&iu=/41014381
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to