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 <pkusunyif...@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 <pkusunyif...@gmail.com> > Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> > > > On Thu, Jul 12, 2018 at 4:33 PM, Ben Pfaff <b...@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 <b...@ovn.org> >> > --- >> > I'm reposting this in hope of getting a review this time. >> >> Still needs review. >> _______________________________________________ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev