Looks good. Does it make sense to do something like this for the other complex actions we have (bundle, autopath, multipath, etc)?
Ethan On Tue, Sep 27, 2011 at 13:36, Ben Pfaff <[email protected]> wrote: > learn_parse() and learn_check() are supposed to apply the same checks, but > they are implemented very differently. It seems best to have learn_parse() > actually call learn_check() just to be sure. > > This would have caught the bug fixed in the previous commit, because > the tests actually contain instances of "learn" actions that trigger the > bug, but the tests only parsed the actions without ever doing anything > that checked them (like sending them to ovs-vswitchd over OpenFlow). > --- > lib/learn.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/lib/learn.c b/lib/learn.c > index 8d7e0b0..9f95a13 100644 > --- a/lib/learn.c > +++ b/lib/learn.c > @@ -414,6 +414,7 @@ learn_parse(struct ofpbuf *b, char *arg, const struct > flow *flow) > char *name, *value; > size_t learn_ofs; > size_t len; > + int error; > > struct nx_action_learn *learn; > struct cls_rule rule; > @@ -497,7 +498,6 @@ learn_parse(struct ofpbuf *b, char *arg, const struct > flow *flow) > } > } > } > - free(orig); > > put_u16(b, 0); > > @@ -508,6 +508,14 @@ learn_parse(struct ofpbuf *b, char *arg, const struct > flow *flow) > > learn = ofpbuf_at_assert(b, learn_ofs, sizeof *learn); > learn->len = htons(b->size - learn_ofs); > + > + /* In theory the above should have caught any errors, but... */ > + error = learn_check(learn, flow); > + if (error) { > + char *msg = ofputil_error_to_string(error); > + ovs_fatal(0, "%s: %s", orig, msg); > + } > + free(orig); > } > > void > -- > 1.7.4.4 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
