[ovs-dev] [PATCH] byte-order: Fix undefined behavior of BYTES_TO_BE32.

[Lance Richardson]

> From: "Ben Pfaff" <blp at ovn.org>
> To: "Lance Richardson" <lrichard at redhat.com>
> Cc: dev at openvswitch.org
> Sent: Tuesday, 13 June, 2017 11:17:26 AM
> Subject: Re: [PATCH] byte-order: Fix undefined behavior of BYTES_TO_BE32.
> 
> On Tue, Jun 13, 2017 at 09:09:44AM -0400, Lance Richardson wrote:
> > > From: "Ben Pfaff" <blp at ovn.org>
> > > To: dev at openvswitch.org
> > > Cc: "Ben Pfaff" <blp at ovn.org>, "Lance Richardson" <lrichard at redhat.com>
> > > Sent: Tuesday, 13 June, 2017 12:51:14 AM
> > > Subject: [PATCH] byte-order: Fix undefined behavior of BYTES_TO_BE32.
> > > 
> > > A left shift that would produce a result that is not representable
> > > by the type of the expression's result has "undefined behavior"
> > > according to the C language standard. Avoid this by casting values
> > > that could set the upper bit to unsigned types.
> > > 
> > > Also document and convert a macro to a function.
> > > 
> > > While we're at it, delete the unused macro BE16S_TO_BE32.
> > > 
> > > Found via gcc's undefined behavior sanitizer.
> > > 
> > > Reported-by: Lance Richardson <lrichard at redhat.com>
> > > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > > ---
> > >  lib/byte-order.h | 21 +++++++++++++--------
> > >  lib/flow.c       |  2 +-
> > >  2 files changed, 14 insertions(+), 9 deletions(-)
> > > 
> > 
> > Looks good.
> > 
> > Acked-by: Lance Richardson <lrichard at redhat.com>
> 
> Thanks.  I applied this to master.
> 
> Do you think that it is worthwhile to apply this to older branches?
> 

I would guess not... the only danger here would be if the compiler
incorrectly optimized something based on inferring that the high-order
bit of the value being shifted 24 bits has to be zero because there
would be "undefined behavior" if it were set. From looking through
the code, there doesn't seem to be any real exposure.

Thanks,

   Lance