Ramesh ,

Thanks for the review .

I will move  `nref_info` &  `cref_info` under condition and I will 
address  the other existing API  mem leaks  , in new bug .

-AVM

On 7/26/2013 1:01 PM, Ramesh Betham wrote:
> Hi Mahesh,
>
> Ack with the following comment. Not tested.
>
> if "/noncoll_rep_on_payload/" is `true', then "/nref_info/" and 
> "/cref_info/" should be removed. (or) malloc "/nref_info/" and 
> "/cref_info/" only when /noncoll_rep_on_payload/" is `false'
>
>
> However there are some corrections required in the existing code, 
> found while reviewing in the patch context.
> -----------------------------------------------------------------------------------------------------------------------
> 1.  In function cpd_sb_proc_ckpt_dest_add()
>
> line: 499
>  else {
>           TRACE_4("cpd standby dest add evt failed for ckptid: 
> %llx",msg->info.dest_add.ckpt_id);
>      return NCSCC_RC_OUT_OF_MEM;
>         }
>
> Memleak:  should free( )   "nref_info" and "cref_info"
>
>
> line: 562
> free_mem:
>         if (node_info == NULL) {
>
>     }
>
> Memleak: If line:513 saClmClusterNodeGet() fails, should free 
> "nref_info" and "cref_info"
>
>
>
> 2. In function cpd_ckpt_reploc_node_delete()
>
> line 437: rc = cpd_ckpt_reploc_node_add(&cb->ckpt_reploc_tree, 
> ckpt_reploc_node, cb->ha_state, cb->immOiHandle);
> Should be removed else..  if "ckpt_reploc_node" has to retain then  do 
> not delete it subsequently.
>
> 3. In function cpd_ckpt_ref_info_add() , "cref_info" is not added to 
> the list if the same "ckpt_id" exist. In this case we have to free() 
> "cref_info".
>
> 4. Similarly, In function cpd_node_ref_info_add() , "nref_info" is not 
> added to the list if the same "dest" exist. In this case we have to 
> free() "nref_info".
> -----------------------------------------------------------------------------------------------------------------------
>
> Regards,
> Ramesh.
>
>
>
>
> On 7/26/2013 9:18 AM, [email protected] wrote:
>>   osaf/services/saf/cpsv/cpd/cpd_proc.c |  54 
>> +++++++++++++++++++++++++---------
>>   1 files changed, 39 insertions(+), 15 deletions(-)
>>
>>
>> diff --git a/osaf/services/saf/cpsv/cpd/cpd_proc.c 
>> b/osaf/services/saf/cpsv/cpd/cpd_proc.c
>> --- a/osaf/services/saf/cpsv/cpd/cpd_proc.c
>> +++ b/osaf/services/saf/cpsv/cpd/cpd_proc.c
>> @@ -148,6 +148,7 @@ uint32_t cpd_ckpt_db_entry_update(CPD_CB
>>      SaClmNodeIdT node_id;
>>      SaNameT ckpt_name;
>>      CPD_REP_KEY_INFO key_info;
>> +    bool noncoll_rep_on_payload = false;
>>   
>>      memset(&ckpt_name, 0, sizeof(SaNameT));
>>      memset(&cluster_node, 0, sizeof(SaClmClusterNodeT));
>> @@ -271,13 +272,34 @@ uint32_t cpd_ckpt_db_entry_update(CPD_CB
>>              }
>>   
>>              *o_ckpt_node = ckpt_node;
>> -                    if (reploc_info && create_reploc_node) {
>> -                    proc_rc = 
>> cpd_ckpt_reploc_node_add(&cb->ckpt_reploc_tree, reploc_info, cb->ha_state, 
>> cb->immOiHandle);
>> -                            if (proc_rc != NCSCC_RC_SUCCESS) {
>> -                             /* goto reploc_node_add_fail; */
>> -                             TRACE_4("cpd db add failed in db entry 
>> update");
>> -                            }
>> -                }
>> +            if (reploc_info && create_reploc_node) {
>> +                    /*According to Ckpt non-collocated ckpt implementation 
>> the cluster can have max 3 replicas
>> +                      and minimum of 2 replicas,if the non-collocated ckpt 
>> is opened on controller initially ,
>> +                      by default cpsv service will create 2 replicas each 
>> one on controllers ,
>> +                      else the non-collocated ckpt is opened on payload 
>> initially,by default cpsv service will create 3 replicas
>> +                      one on the payload and other each one on 
>> controllers,so any further opens form any other payload is not
>> +                      required to create replicas  locally.All other node 
>> ckpt application will access the data form the
>> +                      default created active replica. */
>> +                    if 
>> (m_IS_SA_CKPT_CHECKPOINT_COLLOCATED(&(*io_map_info)->attributes)) {
>> +                            TRACE_4("Reploc node add  for collocated 
>> ckpt_id:%llx",ckpt_id);
>> +                            proc_rc = 
>> cpd_ckpt_reploc_node_add(&cb->ckpt_reploc_tree, reploc_info, cb->ha_state, 
>> cb->immOiHandle);
>> +                            if (proc_rc != NCSCC_RC_SUCCESS) {
>> +                                    /* goto reploc_node_add_fail; */
>> +                                    TRACE_4("cpd db add failed in db entry 
>> update");
>> +                            }
>> +                    } else if 
>> ((cpd_get_slot_sub_id_from_mds_dest(*cpnd_dest) == cb->cpd_remote_id) ||
>> +                            (cpd_get_slot_sub_id_from_mds_dest(*cpnd_dest) 
>> == cb->cpd_self_id) ) {
>> +                            TRACE_4(" reploc node add for non-collocated on 
>> controller ckpt_id:%llx",ckpt_id);
>> +                            proc_rc = 
>> cpd_ckpt_reploc_node_add(&cb->ckpt_reploc_tree, reploc_info, cb->ha_state, 
>> cb->immOiHandle);
>> +                            if (proc_rc != NCSCC_RC_SUCCESS) {
>> +                                    /* goto reploc_node_add_fail; */
>> +                                    TRACE_4("cpd db add failed in db entry 
>> update");
>> +                            }
>> +                    } else {
>> +                            TRACE_4(" reploc node add for non-collocated on 
>> paylaod ckpt_id:%llx",ckpt_id);
>> +                            noncoll_rep_on_payload = true;
>> +                    }
>> +            }
>>      } else {
>>              /* Fill the Map Info */
>>              memset(map_info, 0, sizeof(CPD_CKPT_MAP_INFO));
>> @@ -342,15 +364,17 @@ uint32_t cpd_ckpt_db_entry_update(CPD_CB
>>              *o_ckpt_node = ckpt_node;
>>      }
>>   
>> -    /* Add the CPND Details (CPND reference) to the ckpt node */
>> -    memset(nref_info, 0, sizeof(CPD_NODE_REF_INFO));
>> -    nref_info->dest = *cpnd_dest;
>> -    cpd_node_ref_info_add(ckpt_node, nref_info);
>> +    if (noncoll_rep_on_payload != true) {
>> +            /* Add the CPND Details (CPND reference) to the ckpt node */
>> +            memset(nref_info, 0, sizeof(CPD_NODE_REF_INFO));
>> +            nref_info->dest = *cpnd_dest;
>> +            cpd_node_ref_info_add(ckpt_node, nref_info);
>>   
>> -    /* Add the ckpt reference to the CPND node info */
>> -    memset(cref_info, 0, sizeof(CPD_CKPT_REF_INFO));
>> -    cref_info->ckpt_node = ckpt_node;
>> -    cpd_ckpt_ref_info_add(node_info, cref_info);
>> +            /* Add the ckpt reference to the CPND node info */
>> +            memset(cref_info, 0, sizeof(CPD_CKPT_REF_INFO));
>> +            cref_info->ckpt_node = ckpt_node;
>> +            cpd_ckpt_ref_info_add(node_info, cref_info);
>> +    }
>>   
>>      TRACE_LEAVE();
>>      return NCSCC_RC_SUCCESS;
>

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to