[ovs-dev] [PATCH] ofproto/trace: fix memory leak at recirculate state

[Ben Pfaff]

OK.

Please keep in mind that we'd need {} around the "free" statement here
in any case, because in the OVS style we always bracket even single
statements with {} (see CodingStyle.md).

On Thu, Dec 24, 2015 at 11:30:36PM +0000, ChengChun Tu wrote:
> Hi,
> 
> Please ignore this patch. 
> 
> I tested this patch and found it fails the “make check TESTSUITEFLAGS='381’”. (Surprisingly it passes the “make check-valgrind”, but fails the “make check"). I will investigate it and resubmit. Thank you.
> 
> Regards,
> William
> 
> 
> 
> 
> On 12/24/15, 1:58 PM, "dev on behalf of William Tu" <dev-bounces at openvswitch.org on behalf of u9012063 at gmail.com> wrote:
> 
> >recirc_state_clone() allocates a memory for new->ofpacts without
> >later on freeing it. valgrind test case: 381:
> >    xmemdup(util.c:134)
> >    recirc_state_clone(ofproto-dpif-rid.c:221)
> >    recirc_alloc_id__(ofproto-dpif-rid.c:238)
> >    recirc_alloc_id_ctx(ofproto-dpif-rid.c:281)
> >    compose_recirculate_action__(ofproto-dpif-xlate.c:3643)
> >    compose_recirculate_action(ofproto-dpif-xlate.c:3664)
> >    xlate_actions(ofproto-dpif-xlate.c:5325)
> >    ofproto_trace(ofproto-dpif.c:5074)
> >    ofproto_unixctl_trace(ofproto-dpif.c:4934)
> >    process_command(unixctl.c:297)
> >    run_connection(unixctl.c:331)
> >    unixctl_server_run(unixctl.c:384)
> >    main(ovs-vswitchd.c:121)
> >
> >Signed-off-by: William Tu <u9012063 at gmail.com>
> >---
> > ofproto/ofproto-dpif-rid.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> >diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
> >index 30c1c94..0631f2c 100644
> >--- a/ofproto/ofproto-dpif-rid.c
> >+++ b/ofproto/ofproto-dpif-rid.c
> >@@ -303,6 +303,11 @@ recirc_id_node_unref(const struct recirc_id_node *node_)
> >     OVS_EXCLUDED(mutex)
> > {
> >     struct recirc_id_node *node = CONST_CAST(struct recirc_id_node *, node_);
> >+    const struct recirc_state *state;
> >+
> >+    state = &node->state;
> >+    if (state && state->ofpacts)
> >+        free(state->ofpacts);
> > 
> >     if (node && ovs_refcount_unref(&node->refcount) == 1) {
> >         ovs_mutex_lock(&mutex);
> >-- 
> >2.5.0
> >
> >_______________________________________________
> >dev mailing list
> >dev at openvswitch.org
> >http://openvswitch.org/mailman/listinfo/dev
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev