[ovs-dev] [PATCH v2] ofp-actions: Split ofpacts_check__() into many functions.

[Ben Pfaff]

Thanks.

I applied this patch to master.

On Wed, Jul 18, 2018 at 10:13:05AM -0700, Yifeng Sun wrote:
> Sorry I made a mistake in the previous review. Operator '||' has higher
> precedence
> than operator '?:'. So the patch is correct.
> 
> Thanks,
> Yifeng
> 
> On Tue, Jul 17, 2018 at 1:57 PM, Yifeng Sun <pkusunyifeng at gmail.com> wrote:
> 
> > Hi Ben,
> >
> > I found a small issue:
> >
> > +{
> > +    uint32_t mid = a->meter_id;
> > +    return mid == 0 || mid > OFPM13_MAX ? OFPERR_OFPMMFC_INVALID_METER :
> > 0;
> > +}
> >
> > If mid == 0 is true, then this function return 1, instead of
> > OFPERR_OFPMMFC_INVALID_METER.
> > maybe return (mid == 0 || mid > OFPM13_MAX) ?
> > OFPERR_OFPMMFC_INVALID_METER : 0;
> >
> > Other than that, this patch looks good to me.
> > 'make check' passed all tests.
> >
> > Thanks.
> >
> > Tested-by: Yifeng Sun <pkusunyifeng at gmail.com>
> > Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
> >
> >
> > On Thu, Jul 12, 2018 at 4:33 PM, Ben Pfaff <blp at ovn.org> wrote:
> >
> >> On Fri, Jun 15, 2018 at 04:29:22PM -0700, Ben Pfaff wrote:
> >> > ofpacts_check__() was a huge switch statement with special cases for
> >> many
> >> > different kinds of actions.  This made it unwieldy and put the special
> >> > cases far away from the rest of the code related to a given action.
> >> This
> >> > commit refactors the code to avoid the problem.
> >> >
> >> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> >> > ---
> >> > I'm reposting this in hope of getting a review this time.
> >>
> >> Still needs review.
> >> _______________________________________________
> >> dev mailing list
> >> dev at openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> >
> >