[ovs-dev] [PATCH V2] feature: Create specific types for ofp and odp port

Ben Pfaff blp at nicira.com
Tue Jun 11 23:55:20 UTC 2013


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-alexw at 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 <alexw at 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.



More information about the dev mailing list