On Sun, Apr 23, 2017 at 11:22:03AM -0700, Yi-Hung Wei wrote:
> On Sun, Apr 23, 2017 at 9:56 AM, Ben Pfaff <[email protected]> wrote:
> > On Fri, Apr 07, 2017 at 02:43:40PM -0700, Yi-Hung Wei wrote:
> >> In testcase "ofproto-dpif - fragment handling - actions", valgrind reports
> >> memeory leaks with the following call stack.
> >>     xmalloc (util.c:112)
> >>     xvasprintf (util.c:176)
> >>     xasprintf (util.c:272)
> >>     mf_parse_subfield__ (nx-match.c:1939)
> >>     mf_parse_subfield (nx-match.c:1991)
> >>     learn_parse_spec (learn.c:242)
> >>     learn_parse__ (learn.c:436)
> >>     learn_parse (learn.c:464)
> >>     parse_LEARN (ofp-actions.c:4670)
> >>     ofpact_parse (ofp-actions.c:8231)
> >>     ofpacts_parse__ (ofp-actions.c:8278)
> >>     ofpacts_parse (ofp-actions.c:8350)
> >>     ofpacts_parse_copy (ofp-actions.c:8368)
> >>     parse_ofp_str__ (ofp-parse.c:543)
> >>     parse_ofp_str (ofp-parse.c:596)
> >>     parse_ofp_flow_mod_str (ofp-parse.c:1024)
> >>     ofctl_flow_mod (ovs-ofctl.c:1496)
> >>     ovs_cmdl_run_command__ (command-line.c:115)
> >>     main (ovs-ofctl.c:147)
> >>
> >> Signed-off-by: Yi-Hung Wei <[email protected]>
> >> ---
> >>  lib/learn.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/lib/learn.c b/lib/learn.c
> >> index b7b70ac3d4e2..1c88b8fa402a 100644
> >> --- a/lib/learn.c
> >> +++ b/lib/learn.c
> >> @@ -322,6 +322,7 @@ learn_parse_spec(const char *orig, char *name, char 
> >> *value,
> >>          char *tail;
> >>          char *dst_value = strstr(value, "->");
> >>
> >> +        free(error);
> >>          if (dst_value == value) {
> >>              return xasprintf("%s: missing source before `->' in `%s'", 
> >> name,
> >>                               value);
> >> @@ -358,6 +359,8 @@ learn_parse_spec(const char *orig, char *name, char 
> >> *value,
> >>              spec->dst = move.dst;
> >>          }
> >>      } else if (!strcmp(name, "output")) {
> >> +        free(error);
> >> +
> >>          char *error = mf_parse_subfield(&spec->src, value);
> >>          if (error) {
> >>              return error;
> >> @@ -367,6 +370,7 @@ learn_parse_spec(const char *orig, char *name, char 
> >> *value,
> >>          spec->src_type = NX_LEARN_SRC_FIELD;
> >>          spec->dst_type = NX_LEARN_DST_OUTPUT;
> >>      } else {
> >> +        free(error);
> >>          return xasprintf("%s: unknown keyword %s", orig, name);
> >>      }
> >
> > Thank you for the fix!  That introduces a lot of code duplication.  What
> > do you think of this version?
> >
> > --8<--------------------------cut here-------------------------->8--
> >
> > From: Yi-Hung Wei <[email protected]>
> > Date: Fri, 7 Apr 2017 14:43:40 -0700
> > Subject: [PATCH] learn: Fix memory leak in learn_parse_sepc()
> >
> > In testcase "ofproto-dpif - fragment handling - actions", valgrind reports
> > memeory leaks with the following call stack.
> >     xmalloc (util.c:112)
> >     xvasprintf (util.c:176)
> >     xasprintf (util.c:272)
> >     mf_parse_subfield__ (nx-match.c:1939)
> >     mf_parse_subfield (nx-match.c:1991)
> >     learn_parse_spec (learn.c:242)
> >     learn_parse__ (learn.c:436)
> >     learn_parse (learn.c:464)
> >     parse_LEARN (ofp-actions.c:4670)
> >     ofpact_parse (ofp-actions.c:8231)
> >     ofpacts_parse__ (ofp-actions.c:8278)
> >     ofpacts_parse (ofp-actions.c:8350)
> >     ofpacts_parse_copy (ofp-actions.c:8368)
> >     parse_ofp_str__ (ofp-parse.c:543)
> >     parse_ofp_str (ofp-parse.c:596)
> >     parse_ofp_flow_mod_str (ofp-parse.c:1024)
> >     ofctl_flow_mod (ovs-ofctl.c:1496)
> >     ovs_cmdl_run_command__ (command-line.c:115)
> >     main (ovs-ofctl.c:147)
> >
> > Signed-off-by: Yi-Hung Wei <[email protected]>
> > Signed-off-by: Ben Pfaff <[email protected]>
> > ---
> >  lib/learn.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/learn.c b/lib/learn.c
> > index b7b70ac3d4e2..ebbc210c71ce 100644
> > --- a/lib/learn.c
> > +++ b/lib/learn.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
> > + * Copyright (c) 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -237,10 +237,12 @@ learn_parse_spec(const char *orig, char *name, char 
> > *value,
> >  {
> >      /* Parse destination and check prerequisites. */
> >      struct mf_subfield dst;
> > -    char *error;
> >
> > -    error = mf_parse_subfield(&dst, name);
> > -    if (!error) {
> > +    char *error = mf_parse_subfield(&dst, name);
> > +    bool parse_error = error != NULL;
> > +    free(error);
> > +
> > +    if (parse_error) {
> 
> Thanks, it looks much better. It seems like there is a typo here.
> "  if (!parse_error) { "

You're right.  Thanks, I fixed that and I applied this to master and
branch-2.7.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to