diff mbox

[ovs-dev] test-aa: fix memory leak reported by valgrind

Message ID 1452134123-14579-1-git-send-email-u9012063@gmail.com
State Accepted
Headers show

Commit Message

William Tu Jan. 7, 2016, 2:35 a.m. UTC 
test case 1698: auto-attach - packet tests
Report several leaks at lldp_create_dummy(), the patch fixes the
following 3 leaks:
    {lldp_send (lldp.c:334), lldp_decode (lldp.c:374),
     lldp_create_dummy (ovs-lldp.c:890)}
    test_aa_send (test-aa.c:252)
    test_aa_main (test-aa.c:281)
Comments:
    1. Create a new function "lldp_destroy_dummy()" because
       many structures and its elements, ex: lldp_hardware and lldp_chassis,
       are from stack not heap (see test_aa_send). As a result, calling
       lldpd_cleanup() is incorrect.
    2. Remove lchassis->c_id = xmalloc(ETH_ADDR_LEN);
       because it is overwritten at test_aa_send()
    3. remove memcpy(&hw->h_lport.p_element.system_id.system_mac,
           lchassis->c_id, lchassis->c_id_len);
       because the source buf is empty

Signed-off-by: William Tu <u9012063@gmail.com>
---
 lib/ovs-lldp.c  | 34 ++++++++++++++++++++++++++++++----
 lib/ovs-lldp.h  |  1 +
 tests/test-aa.c |  5 +++++
 3 files changed, 36 insertions(+), 4 deletions(-)

Comments

Ben Pfaff Jan. 11, 2016, 5:02 p.m. UTC | #1 
On Wed, Jan 06, 2016 at 06:35:23PM -0800, William Tu wrote:
> test case 1698: auto-attach - packet tests
> Report several leaks at lldp_create_dummy(), the patch fixes the
> following 3 leaks:
>     {lldp_send (lldp.c:334), lldp_decode (lldp.c:374),
>      lldp_create_dummy (ovs-lldp.c:890)}
>     test_aa_send (test-aa.c:252)
>     test_aa_main (test-aa.c:281)
> Comments:
>     1. Create a new function "lldp_destroy_dummy()" because
>        many structures and its elements, ex: lldp_hardware and lldp_chassis,
>        are from stack not heap (see test_aa_send). As a result, calling
>        lldpd_cleanup() is incorrect.
>     2. Remove lchassis->c_id = xmalloc(ETH_ADDR_LEN);
>        because it is overwritten at test_aa_send()
>     3. remove memcpy(&hw->h_lport.p_element.system_id.system_mac,
>            lchassis->c_id, lchassis->c_id_len);
>        because the source buf is empty
> 
> Signed-off-by: William Tu <u9012063@gmail.com>

Thanks, applied to master.
diff mbox

Patch

diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
index bd1729c..16225b5 100644
--- a/lib/ovs-lldp.c
+++ b/lib/ovs-lldp.c
@@ -887,7 +887,6 @@  lldp_create_dummy(void)
     lchassis->c_cap_enabled = LLDP_CAP_BRIDGE;
     lchassis->c_id_subtype = LLDP_CHASSISID_SUBTYPE_LLADDR;
     lchassis->c_id_len = ETH_ADDR_LEN;
-    lchassis->c_id = xmalloc(ETH_ADDR_LEN);
 
     list_init(&lchassis->c_mgmt);
     lchassis->c_ttl = LLDP_CHASSIS_TTL;
@@ -911,8 +910,6 @@  lldp_create_dummy(void)
     /* Auto Attach element tlv */
     hw->h_lport.p_element.type = LLDP_TLV_AA_ELEM_TYPE_CLIENT_VIRTUAL_SWITCH;
     hw->h_lport.p_element.mgmt_vlan = 0;
-    memcpy(&hw->h_lport.p_element.system_id.system_mac,
-           lchassis->c_id, lchassis->c_id_len);
     hw->h_lport.p_element.system_id.conn_type =
         LLDP_TLV_AA_ELEM_CONN_TYPE_SINGLE;
     hw->h_lport.p_element.system_id.rsvd = 0;
@@ -950,7 +947,7 @@  lldp_unref(struct lldp *lldp)
     free(lldp);
 }
 
-/* Unreference a specific LLDP instance.
+/* Reference a specific LLDP instance.
  */
 struct lldp *
 lldp_ref(const struct lldp *lldp_)
@@ -961,3 +958,32 @@  lldp_ref(const struct lldp *lldp_)
     }
     return lldp;
 }
+
+void
+lldp_destroy_dummy(struct lldp *lldp)
+{
+    struct lldpd_hardware *hw, *hw_next;
+    struct lldpd_chassis *chassis, *chassis_next;
+    struct lldpd *cfg;
+
+    if (!lldp) {
+        return;
+    }
+
+    cfg = lldp->lldpd;
+
+    LIST_FOR_EACH_SAFE (hw, hw_next, h_entries, &cfg->g_hardware) {
+        list_remove(&hw->h_entries);
+        free(hw->h_lport.p_lastframe);
+        free(hw);
+    }
+
+    LIST_FOR_EACH_SAFE (chassis, chassis_next, list, &cfg->g_chassis) {
+        list_remove(&chassis->list);
+        free(chassis);
+    }
+
+    free(lldp->lldpd);
+    free(lldp);
+}
+
diff --git a/lib/ovs-lldp.h b/lib/ovs-lldp.h
index e780e5b..71dff44 100644
--- a/lib/ovs-lldp.h
+++ b/lib/ovs-lldp.h
@@ -103,5 +103,6 @@  int aa_mapping_unregister(void *aux);
 
 /* Used by unit tests */
 struct lldp * lldp_create_dummy(void);
+void lldp_destroy_dummy(struct lldp *);
 
 #endif /* OVS_LLDP_H */
diff --git a/tests/test-aa.c b/tests/test-aa.c
index 2da572d..eedefeb 100644
--- a/tests/test-aa.c
+++ b/tests/test-aa.c
@@ -267,6 +267,11 @@  test_aa_send(void)
     /* Verify auto attach values */
     check_received_aa(&hardware.h_lport, nport, map_init);
 
+    lldpd_chassis_cleanup(nchassis, true);
+    lldpd_port_cleanup(nport, true);
+    free(nport);
+    lldp_destroy_dummy(lldp);
+
     return 0;
 }