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

Ben Pfaff blp at nicira.com
Thu Jun 20 19:47:42 UTC 2013


On Thu, Jun 20, 2013 at 12:35:57PM -0700, Alex Wang wrote:
> And sorry, I should have put you as co-author in the last commit.

I don't think it was warranted, you did most of the work.

> On Thu, Jun 20, 2013 at 10:53 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Wed, Jun 19, 2013 at 04:58:44PM -0700, Alex Wang wrote:
> > > Currently datapath ports and openflow ports are both represented by
> > unsigned
> > > integers of various sizes. With implicit casts, etc. it is easy to mix
> > them
> > > up and use one where the other is expected. This commit creates two
> > typedefs
> > > ofp_port_t and odp_port_t. Both of these two types are marked by
> > > "__attribute__((bitwise))" so that Sparse can be used to detect any
> > misuse.
> > >
> > > Signed-off-by: Alex Wang <alexw at nicira.com>
> >
> > I folded in some incrementals, below, and pushed this to master.
> >
> > The incrementals:
> >
> >         * Better protect the *_PORT_C macros.  I'd meant them only for
> >           integer literals (like UINT32_C, etc.) so I didn't
> >           originally put enough parentheses in, but I can see that it
> >           is tempting to use them in other places.
> >
> 
> 
> I really want to learn the difference here. Could you please explain more
> why the additional parentheses matter?

Casts have higher precedence that most other operators, so (type)x and
(type)(x) can be different.  For example, (uint32_t)UINT64_MAX >> 1 has
value UINT32_MAX/2, but (uint32_t)(UINT64_MAX >> 1) has value
UINT32_MAX, if I haven't screwed anything up.

> >         * Fix some places where UINT16_MAX was used for a mask but it
> >           had been replaced by OFPP_MAX.  It's important for a mask
> >           that it be all-1-bits, so UINT16_MAX is appropriate there.
>> 
> Actually, I think you mean OFPP_NONE here, right? 

Oops, yes.

> Is it to be more clear that we use "u16_to_ofp(UINT16_MAX)" for
> "masks.in_port.ofp_port" assignment ?

Yes, thanks for the correction.



More information about the dev mailing list