Message ID | 1450994285-40316-1-git-send-email-u9012063@gmail.com |
---|---|
State | Awaiting Upstream |
Headers | show |
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@openvswitch.org on behalf of u9012063@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@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@openvswitch.org >http://openvswitch.org/mailman/listinfo/dev
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@openvswitch.org on behalf of u9012063@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@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@openvswitch.org > >http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
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);
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@gmail.com> --- ofproto/ofproto-dpif-rid.c | 5 +++++ 1 file changed, 5 insertions(+)