Thanks!
On Tue, Jun 11, 2013 at 05:03:46PM -0700, Alex Wang wrote:
> Thanks Ben,
>
> The factors are all reasonable. Especially, the negation can be confusing.
>
> Also, with you confirmation, I'll choose the "oft_to_u16()", "odp_to_u32()"
> way. ;D
>
> I'll adjust the code accordingly and resend a new version.
>
> Kind Regards,
> Alex Wang,
>
>
> On Tue, Jun 11, 2013 at 4:55 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> > On Mon, Jun 10, 2013 at 08:50:40AM -0700, Alex Wang wrote:
> > > Subject: [PATCH] This patch implements "ofp_is_less_than()" and use it
> > > to replace the PORT_COMPARE() function for port comparison.
> > > Date: Mon, 10 Jun 2013 09:05:50 -0700
> > > Message-Id: <1370880350-22000-1-git-send-email-al...@nicira.com>
> > > X-Mailer: git-send-email 1.7.9.5
> > >
> > > This patch implements "ofp_is_less_than()" and use it to replace the
> > > PORT_COMPARE() function for port comparison.
> > >
> > > Signed-off-by: Alex Wang <al...@nicira.com>
> >
> > I'm not a fan of this approach. It's hard to put my finger on exactly
> > why, but some factors that I don't like are:
> >
> > * For me, anyway, it's hard to think through the ordering and
> > (sometimes) negation to figure out whether the condition is
> > correct.
> >
> > * The form is different from a normal comparison, which makes
> > it a little harder to read.
> >
> > * It's pretty long.
> >
> > I actually find the PORT_COMPARE form easier to read, although it
> > definitely has downsides of its own.
> >
> > Here's another form that I find a little easier to read (which might
> > only be because I've used it fairly often before):
> >
> > static inline int
> > ofp_compare_3way(ofp_port_t a_, ofp_port_t b_)
> > {
> > uint32_t a = (OVS_FORCE uint32_t) a_;
> > uint32_t b = (OVS_FORCE uint32_t) b_;
> > return a < b ? -1 : a > b;
> > }
> >
> > Then you can write any condition you like by comparing against 0. For
> > example:
> > if (ofp_compare_3way(ofp_port, OFPP_MAX) >= 0)
> > is equivalent to:
> > if (ofp_port >= OFPP_MAX)
> > and
> > if (ofp_compare_3way(ofp_port, OFPP_MAX) < 0)
> > is equivalent to:
> > if (ofp_port < OFPP_MAX)
> >
> > But I think I still like the way I proposed earlier better.
> >
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
