[ovs-dev] [PATCH V3] feature: Create specific types for ofp and odp port
alexw at nicira.com
Thu Jun 20 21:48:59 UTC 2013
Thanks for the explanation,
On Thu, Jun 20, 2013 at 12:47 PM, Ben Pfaff <blp at nicira.com> wrote:
> 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
> > > 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.
This makes sense. Thanks,
> > > * 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.
Then, there is one leftover in tests/test-classifier.c, I'll patch it
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the dev