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