Hi Thuan,

I will update in V2.

Best Regards,
ThienHuynh
________________________________
From: Thuan Tran <thuan.t...@dektech.com.au>
Sent: Tuesday, July 28, 2020 6:31 PM
To: Thang Duc Nguyen <than...@users.sourceforge.net>; Thien Minh Huynh 
<thien.m.hu...@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net <opensaf-devel@lists.sourceforge.net>
Subject: Re: [devel] [PATCH 1/1] clm: fix memory leak reeported by valgrind 
[#3206]

Hi Thien,

See my reply.

Best Regards,
Thuan


________________________________
From: Thien Minh Huynh <thien.m.hu...@dektech.com.au>
Sent: Tuesday, July 28, 2020, 17:56
To: Thuan Tran; Thang Duc Nguyen
Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [devel] [PATCH 1/1] clm: fix memory leak reeported by valgrind 
[#3206]

Hi Thuan,

Thanks for your comments. See a comment inline.

Best Regards,
ThienHuynh

-----Original Message-----
From: Thuan Tran <thuan.t...@dektech.com.au>
Sent: Tuesday, July 28, 2020 4:59 PM
To: Thien Minh Huynh <thien.m.hu...@dektech.com.au>; Thang Duc Nguyen 
<than...@users.sourceforge.net>; Thuan Tran <thua...@users.sourceforge.net>
Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: [devel] [PATCH 1/1] clm: fix memory leak reeported by valgrind 
[#3206]

Hi Thien,

Good work! Some comments inline.
Also, commit message typo: "reeported" -> "reported"

Best Regards,
ThuanTr

-----Original Message-----
From: thien.m.huynh <thien.m.hu...@dektech.com.au>
Sent: Tuesday, July 28, 2020 9:27 AM
To: Thang Duc Nguyen <than...@users.sourceforge.net>; Thuan Tran 
<thua...@users.sourceforge.net>
Cc: opensaf-devel@lists.sourceforge.net
Subject: [devel] [PATCH 1/1] clm: fix memory leak reeported by valgrind [#3206]

Fix definitely lost reported by valgrind.
---
 src/clm/clmd/clms_evt.cc   |  5 +----
 src/clm/clmd/clms_mbcsv.cc | 10 ++++++++++
 src/clm/clmd/clms_mds.cc   |  2 ++
 src/mbc/mbcsv_mds.c        |  3 +++
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/clm/clmd/clms_evt.cc b/src/clm/clmd/clms_evt.cc index 
28f5596ec..e8577060e 100644
--- a/src/clm/clmd/clms_evt.cc
+++ b/src/clm/clmd/clms_evt.cc
@@ -2136,10 +2136,7 @@ void clms_remove_node_down_rec(SaClmNodeIdT node_id) {
  */
 void clms_evt_destroy(CLMSV_CLMS_EVT *evt) {
   osafassert(evt != nullptr);
-  if (evt->info.msg.info.api_info.type == CLMSV_CLUSTER_JOIN_REQ) {
-    TRACE("not calloced in server code,don't free it here");
-  } else
-    free(evt);
+  free(evt);
 }

 /*clms ack to response msg from clma
diff --git a/src/clm/clmd/clms_mbcsv.cc b/src/clm/clmd/clms_mbcsv.cc index 
049b32c41..aa86b46c5 100644
--- a/src/clm/clmd/clms_mbcsv.cc
+++ b/src/clm/clmd/clms_mbcsv.cc
@@ -2092,6 +2092,9 @@ static uint32_t ckpt_decode_cold_sync(CLMS_CB *cb, 
NCS_MBCSV_CB_ARG *cbk_arg) {
   CLMS_CKPT_REC msg;
   CLMS_CKPT_REC *data = nullptr;
   uint32_t num_rec = 0;
+  uint8_t data_cnt[16];
+  uint32_t num_of_async_upd;
+  uint8_t *ptr;
   TRACE_ENTER();

   /*
@@ -2188,6 +2191,13 @@ static uint32_t ckpt_decode_cold_sync(CLMS_CB *cb, 
NCS_MBCSV_CB_ARG *cbk_arg) {
     --num_rec;
   }

+  /* Get the async update count */
+  ptr = ncs_dec_flatten_space(&cbk_arg->info.decode.i_uba, data_cnt,
+                              sizeof(uint32_t));  num_of_async_upd =
+ ncs_decode_32bit(&ptr);  cb->async_upd_cnt = num_of_async_upd;
+ ncs_dec_skip_space(&cbk_arg->info.decode.i_uba, sizeof(uint32_t));
+
[Thuan] Can be shorten like:
-------
      ptr = ncs_dec_flatten_space(&cbk_arg->info.decode.i_uba,
                        (uint8_t*)&cb->async_upd_cnt, sizeof(uint32_t));
      cb->async_upd_cnt = ncs_decode_32bit(&ptr);
-------
[Thien]
If without "ncs_dec_skip_space(&cbk_arg->info.decode.i_uba, sizeof(uint32_t));"
memory still on leak 4byte.
[Thuan] I just miss that line, I don't mean to remove that line.
--------

 done:
   if (rc != NCSCC_RC_SUCCESS) {
     /* Do not allow standby to get out of sync */ diff --git 
a/src/clm/clmd/clms_mds.cc b/src/clm/clmd/clms_mds.cc index 
5a77885ac..45d1453e4 100644
--- a/src/clm/clmd/clms_mds.cc
+++ b/src/clm/clmd/clms_mds.cc
@@ -1247,6 +1247,8 @@ static uint32_t clms_mds_svc_event(struct 
ncsmds_callback_info *info) {
         rc = NCSCC_RC_FAILURE;
         goto done;
       }
+    } else {
+      free(evt);
     }
   }
 done:
diff --git a/src/mbc/mbcsv_mds.c b/src/mbc/mbcsv_mds.c index 
f3370f8e2..efb86c42a 100644
--- a/src/mbc/mbcsv_mds.c
+++ b/src/mbc/mbcsv_mds.c
@@ -391,6 +391,9 @@ uint32_t mbcsv_mds_rcv(NCSMDS_CALLBACK_INFO *cbinfo)
                         TRACE_LEAVE2("ipc send failed");
                         return NCSCC_RC_FAILURE;
                 }
+       } else {
+               if (msg)
+                       m_MMGR_FREE_MBCSV_EVT(msg);
[Thuan] Can be shorten like:
-------
            } else {
                    m_MMGR_FREE_MBCSV_EVT(msg);
            }
-------

         }

         return NCSCC_RC_SUCCESS;
--
2.17.1



_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to