From: Bill Fischofer [mailto:[email protected]] Sent: Thursday, July 07, 2016 2:42 PM To: Savolainen, Petri (Nokia - FI/Espoo) <[email protected]> Cc: [email protected] Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions
On Thu, Jul 7, 2016 at 2:46 AM, Savolainen, Petri (Nokia - FI/Espoo) <[email protected]> wrote: I'm OK with the change, but didn't do it as part of my patch set since it's an API change. I think the change is not a big deal and should be done to align the usage of byte/bitfield defines. The patch should be tagged with api: prefix and go through api-next. Also, API spec could be more explicit about the byte/bitfield defines and how application should use/test those. I think bitfield defines should work the same way as the current byte order defines. #if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN) // big endian byte order #else // little endian byte order #endif -Petri Not making an API change was the reason I did this change this way. This is now an implementation change only. The change that would involve an API change would have been to define an ODP_BITFIELD_ORDER variable to mirror the ODP_BYTE_ORDER variable and set that to either ODP_LITTLE_ENDIAN_BITFIELD or ODP_BIG_ENDIAN_BITFIELD as needed. > --- a/platform/linux-generic/include/protocols/tcp.h > +++ b/platform/linux-generic/include/protocols/tcp.h > @@ -34,7 +34,7 @@ typedef struct ODP_PACKED { > odp_u32be_t ack_no; /**< Acknowledgment number */ > union { > odp_u16be_t doffset_flags; > -#if defined(ODP_BIG_ENDIAN_BITFIELD) > +#if ODP_BIG_ENDIAN_BITFIELD The change is non-backward, API change for application code. Tcp helper is also application code. Today, application may do this half of the time ... #if defined(ODP_BIG_ENDIAN_BITFIELD) // foo #else // bar #endif ... and this the other half ... #if defined(ODP_LITTLE_ENDIAN_BITFIELD) // foo #else // bar #endif After the change, application would break since both ODP_LITTLE_ENDIAN_BITFIELD and ODP_BIG_ENDIAN_BITFIELD are defined always. -Petri
