Ack with comments inline, marked AndersW>

regards,

Anders Widell


On 01/04/2017 03:28 PM, Zoran Milinkovic wrote:
>   src/base/sysf_tmr.c |  9 ++++++++-
>   1 files changed, 8 insertions(+), 1 deletions(-)
>
>
> Fix memory leak in ncs_tmr_add_pat_node()
> Fix timer state, so that the timer will not execute if ncs_tmr_start() fails
>
> diff --git a/src/base/sysf_tmr.c b/src/base/sysf_tmr.c
> --- a/src/base/sysf_tmr.c
> +++ b/src/base/sysf_tmr.c
> @@ -180,6 +180,7 @@ static struct timespec ts_start;
>   static uint32_t ncs_tmr_add_pat_node(SYSF_TMR *tmr)
>   {
>       SYSF_TMR_PAT_NODE *temp_tmr_pat_node = NULL;
> +     int rc;
AndersW> should be "unsigned int" to match the return type of 
ncs_patricia_tree_add()
>   
>       temp_tmr_pat_node = (SYSF_TMR_PAT_NODE 
> *)ncs_patricia_tree_get(&gl_tcb.tmr_pat_tree, (uint8_t *)&tmr->key);
>   
> @@ -190,7 +191,11 @@ static uint32_t ncs_tmr_add_pat_node(SYS
>               memset(temp_tmr_pat_node, '\0', sizeof(SYSF_TMR_PAT_NODE));
>               temp_tmr_pat_node->key = tmr->key;
>               temp_tmr_pat_node->pat_node.key_info = (uint8_t 
> *)&temp_tmr_pat_node->key;
> -             ncs_patricia_tree_add(&gl_tcb.tmr_pat_tree, (NCS_PATRICIA_NODE 
> *)&temp_tmr_pat_node->pat_node);
> +             rc = ncs_patricia_tree_add(&gl_tcb.tmr_pat_tree, 
> (NCS_PATRICIA_NODE *)&temp_tmr_pat_node->pat_node);
> +             if(rc != NCSCC_RC_SUCCESS) {
> +                     m_NCS_MEM_FREE(temp_tmr_pat_node, 
> NCS_MEM_REGION_PERSISTENT, NCS_SERVICE_ID_LEAP_TMR, 0);
> +                     return NCSCC_RC_FAILURE;
> +             }
>       }
>   
>       if (temp_tmr_pat_node->tmr_list_start == NULL) {
> @@ -733,6 +738,8 @@ tmr_t ncs_tmr_start(tmr_t tid, int64_t t
>                  on the "sel_obj".  */
>               if (m_NCS_SEL_OBJ_IND(&gl_tcb.sel_obj) != NCSCC_RC_SUCCESS) {
>                       /* We would never reach here! */
> +                     /* Ignore this timer. Indication was not successfully 
> sent */
> +                     TMR_SET_STATE(tmr, TMR_STATE_DESTROY);
AndersW> I think it is better to call osaf_abort() in this error case, 
since there must be some serious problem. Setting the timer state to 
TMR_STATE_DESTROY could be dangerous - doesn't that mean that the timer 
could be re-used by someone else?
>                       m_NCS_UNLOCK(&gl_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>                       m_LEAP_DBG_SINK_VOID;
>                       return NULL;
>
> ------------------------------------------------------------------------------
> 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
>


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to