If it can't be done in a backwards compatible way this has to go to TigerMoth.
Do we patch in Monarch and fix properly there ? Mike On 7 July 2016 at 09:48, Bill Fischofer <[email protected]> wrote: > On Thu, Jul 7, 2016 at 8:38 AM, Savolainen, Petri (Nokia - FI/Espoo) < > [email protected]> wrote: > > > > > > > From: Bill Fischofer [mailto:[email protected]] > > Sent: Thursday, July 07, 2016 3:01 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 6:57 AM, Savolainen, Petri (Nokia - FI/Espoo) < > > [email protected]> wrote: > > > > > > 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. > > > > Yes, which is why the patch includes an update to both copies of tcp.h, > > which are currently the only files that reference those #defines. > > > > The issue is that the recent change seems to have broken doxygen, so the > > question is what's the best way to fix that? > > > > > > << Reply to HTML mail>> > > You must consider helpers as part of application code. There may be other > > application code out there which does exactly the same thing (the #if > > defined example above). This change would break *any* application code > that > > uses the defines. > > > > So short of reverting your latest header restructure patch, how would you > suggest fixing this? > > > > > > -Petri > > > > > > > > > > > > > > > > > -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
