| Message ID | 1441306762-22068-1-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | Accepted |
| Commit | 3afd410eaa2e55f47b42508ac0b86390a7b4c711 |
| Headers | show |
On 3 September 2015 at 14:59, Bill Fischofer <bill.fischofer@linaro.org> wrote: > With the addition of ordered queues, there is a circular typedef > relationship between odp_queue_internal.h and odp_buffer_internal.h. > The standard forward declaration technique that GCC accepts is strictly > not acceptable to C99 and is flagged by clang. The solution is to create > a common header file that can contain these forward declarations. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > Reviewed-by: Mike Holmes > --- > platform/linux-generic/Makefile.am | 1 + > .../linux-generic/include/odp_buffer_internal.h | 11 ++------ > .../include/odp_forward_typedefs_internal.h | 32 > ++++++++++++++++++++++ > .../linux-generic/include/odp_queue_internal.h | 5 ++-- > 4 files changed, 39 insertions(+), 10 deletions(-) > create mode 100644 > platform/linux-generic/include/odp_forward_typedefs_internal.h > > diff --git a/platform/linux-generic/Makefile.am > b/platform/linux-generic/Makefile.am > index f2c081a..4c79730 100644 > --- a/platform/linux-generic/Makefile.am > +++ b/platform/linux-generic/Makefile.am > @@ -122,6 +122,7 @@ noinst_HEADERS = \ > ${srcdir}/include/odp_classification_internal.h \ > ${srcdir}/include/odp_crypto_internal.h \ > ${srcdir}/include/odp_debug_internal.h \ > + ${srcdir}/include/odp_forward_typedefs_internal.h \ > ${srcdir}/include/odp_internal.h \ > ${srcdir}/include/odp_packet_internal.h \ > ${srcdir}/include/odp_packet_io_internal.h \ > diff --git a/platform/linux-generic/include/odp_buffer_internal.h > b/platform/linux-generic/include/odp_buffer_internal.h > index 6badeba..4cacca1 100644 > --- a/platform/linux-generic/include/odp_buffer_internal.h > +++ b/platform/linux-generic/include/odp_buffer_internal.h > @@ -29,7 +29,7 @@ extern "C" { > #include <odp/byteorder.h> > #include <odp/thread.h> > #include <odp/event.h> > - > +#include <odp_forward_typedefs_internal.h> > > #define ODP_BITSIZE(x) \ > ((x) <= 2 ? 1 : \ > @@ -101,13 +101,8 @@ typedef union odp_buffer_bits_t { > }; > } odp_buffer_bits_t; > > -/* forward declaration */ > -struct odp_buffer_hdr_t; > -union queue_entry_u; > -typedef union queue_entry_u queue_entry_t; > - > /* Common buffer header */ > -typedef struct odp_buffer_hdr_t { > +struct odp_buffer_hdr_t { > struct odp_buffer_hdr_t *next; /* next buf in a list--keep > 1st */ > union { /* Multi-use secondary link */ > struct odp_buffer_hdr_t *prev; > @@ -144,7 +139,7 @@ typedef struct odp_buffer_hdr_t { > queue_entry_t *target_qe; /* ordered queue target */ > uint64_t sync; /* for ordered > synchronization */ > }; > -} odp_buffer_hdr_t; > +}; > > /** @internal Compile time assert that the > * allocator field can handle any allocator id*/ > diff --git > a/platform/linux-generic/include/odp_forward_typedefs_internal.h > b/platform/linux-generic/include/odp_forward_typedefs_internal.h > new file mode 100644 > index 0000000..f8832f7 > --- /dev/null > +++ b/platform/linux-generic/include/odp_forward_typedefs_internal.h > @@ -0,0 +1,32 @@ > +/* Copyright (c) 2015, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP forward typedefs - implementation internal > + * > + * This needs to be a separate file because it is needed by both > + * odp_queue_internal.h and odp_buffer_internal.h and clang prohibits > forward > + * "redefining" typedefs. Note that this file can be extended with > additional > + * forward typedefs as needed. > + */ > Nit: I think this would be cleaner just saying the following to save editing when updated with a new define. Contains forward typedefs when multiple files need the same definition. > + > +#ifndef ODP_FORWARD_TYPEDEFS_INTERNAL_H_ > +#define ODP_FORWARD_TYPEDEFS_INTERNAL_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +typedef struct odp_buffer_hdr_t odp_buffer_hdr_t; > +typedef union queue_entry_u queue_entry_t; > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > diff --git a/platform/linux-generic/include/odp_queue_internal.h > b/platform/linux-generic/include/odp_queue_internal.h > index 0f30965..19a0f07 100644 > --- a/platform/linux-generic/include/odp_queue_internal.h > +++ b/platform/linux-generic/include/odp_queue_internal.h > @@ -19,6 +19,7 @@ extern "C" { > #endif > > #include <odp/queue.h> > +#include <odp_forward_typedefs_internal.h> > #include <odp_buffer_internal.h> > #include <odp_align_internal.h> > #include <odp/packet_io.h> > @@ -86,10 +87,10 @@ struct queue_entry_s { > odp_atomic_u64_t sync_out; > }; > > -typedef union queue_entry_u { > +union queue_entry_u { > struct queue_entry_s s; > uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(struct > queue_entry_s))]; > -} queue_entry_t; > +}; > > > queue_entry_t *get_qentry(uint32_t queue_id); > -- > 2.1.4 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
The comments in the file say that. I'd assume each commit message as this file grows would describe the specific reason for its changes. On Thu, Sep 3, 2015 at 2:14 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > > > On 3 September 2015 at 14:59, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> With the addition of ordered queues, there is a circular typedef >> relationship between odp_queue_internal.h and odp_buffer_internal.h. >> The standard forward declaration technique that GCC accepts is strictly >> not acceptable to C99 and is flagged by clang. The solution is to create >> a common header file that can contain these forward declarations. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> > > Reviewed-by: Mike Holmes > > >> --- >> platform/linux-generic/Makefile.am | 1 + >> .../linux-generic/include/odp_buffer_internal.h | 11 ++------ >> .../include/odp_forward_typedefs_internal.h | 32 >> ++++++++++++++++++++++ >> .../linux-generic/include/odp_queue_internal.h | 5 ++-- >> 4 files changed, 39 insertions(+), 10 deletions(-) >> create mode 100644 >> platform/linux-generic/include/odp_forward_typedefs_internal.h >> >> diff --git a/platform/linux-generic/Makefile.am >> b/platform/linux-generic/Makefile.am >> index f2c081a..4c79730 100644 >> --- a/platform/linux-generic/Makefile.am >> +++ b/platform/linux-generic/Makefile.am >> @@ -122,6 +122,7 @@ noinst_HEADERS = \ >> ${srcdir}/include/odp_classification_internal.h \ >> ${srcdir}/include/odp_crypto_internal.h \ >> ${srcdir}/include/odp_debug_internal.h \ >> + ${srcdir}/include/odp_forward_typedefs_internal.h \ >> ${srcdir}/include/odp_internal.h \ >> ${srcdir}/include/odp_packet_internal.h \ >> ${srcdir}/include/odp_packet_io_internal.h \ >> diff --git a/platform/linux-generic/include/odp_buffer_internal.h >> b/platform/linux-generic/include/odp_buffer_internal.h >> index 6badeba..4cacca1 100644 >> --- a/platform/linux-generic/include/odp_buffer_internal.h >> +++ b/platform/linux-generic/include/odp_buffer_internal.h >> @@ -29,7 +29,7 @@ extern "C" { >> #include <odp/byteorder.h> >> #include <odp/thread.h> >> #include <odp/event.h> >> - >> +#include <odp_forward_typedefs_internal.h> >> >> #define ODP_BITSIZE(x) \ >> ((x) <= 2 ? 1 : \ >> @@ -101,13 +101,8 @@ typedef union odp_buffer_bits_t { >> }; >> } odp_buffer_bits_t; >> >> -/* forward declaration */ >> -struct odp_buffer_hdr_t; >> -union queue_entry_u; >> -typedef union queue_entry_u queue_entry_t; >> - >> /* Common buffer header */ >> -typedef struct odp_buffer_hdr_t { >> +struct odp_buffer_hdr_t { >> struct odp_buffer_hdr_t *next; /* next buf in a list--keep >> 1st */ >> union { /* Multi-use secondary link >> */ >> struct odp_buffer_hdr_t *prev; >> @@ -144,7 +139,7 @@ typedef struct odp_buffer_hdr_t { >> queue_entry_t *target_qe; /* ordered queue target */ >> uint64_t sync; /* for ordered >> synchronization */ >> }; >> -} odp_buffer_hdr_t; >> +}; >> >> /** @internal Compile time assert that the >> * allocator field can handle any allocator id*/ >> diff --git >> a/platform/linux-generic/include/odp_forward_typedefs_internal.h >> b/platform/linux-generic/include/odp_forward_typedefs_internal.h >> new file mode 100644 >> index 0000000..f8832f7 >> --- /dev/null >> +++ b/platform/linux-generic/include/odp_forward_typedefs_internal.h >> @@ -0,0 +1,32 @@ >> +/* Copyright (c) 2015, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +/** >> + * @file >> + * >> + * ODP forward typedefs - implementation internal >> + * >> + * This needs to be a separate file because it is needed by both >> + * odp_queue_internal.h and odp_buffer_internal.h and clang prohibits >> forward >> + * "redefining" typedefs. Note that this file can be extended with >> additional >> + * forward typedefs as needed. >> + */ >> > > Nit: I think this would be cleaner just saying the following to save > editing when updated with a new define. > > Contains forward typedefs when multiple files need the same definition. > > > >> + >> +#ifndef ODP_FORWARD_TYPEDEFS_INTERNAL_H_ >> +#define ODP_FORWARD_TYPEDEFS_INTERNAL_H_ >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +typedef struct odp_buffer_hdr_t odp_buffer_hdr_t; >> +typedef union queue_entry_u queue_entry_t; >> + >> +#ifdef __cplusplus >> +} >> +#endif >> + >> +#endif >> diff --git a/platform/linux-generic/include/odp_queue_internal.h >> b/platform/linux-generic/include/odp_queue_internal.h >> index 0f30965..19a0f07 100644 >> --- a/platform/linux-generic/include/odp_queue_internal.h >> +++ b/platform/linux-generic/include/odp_queue_internal.h >> @@ -19,6 +19,7 @@ extern "C" { >> #endif >> >> #include <odp/queue.h> >> +#include <odp_forward_typedefs_internal.h> >> #include <odp_buffer_internal.h> >> #include <odp_align_internal.h> >> #include <odp/packet_io.h> >> @@ -86,10 +87,10 @@ struct queue_entry_s { >> odp_atomic_u64_t sync_out; >> }; >> >> -typedef union queue_entry_u { >> +union queue_entry_u { >> struct queue_entry_s s; >> uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(struct >> queue_entry_s))]; >> -} queue_entry_t; >> +}; >> >> >> queue_entry_t *get_qentry(uint32_t queue_id); >> -- >> 2.1.4 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs > > >
Merged, Maxim. On 09/03/15 21:59, Bill Fischofer wrote: > With the addition of ordered queues, there is a circular typedef > relationship between odp_queue_internal.h and odp_buffer_internal.h. > The standard forward declaration technique that GCC accepts is strictly > not acceptable to C99 and is flagged by clang. The solution is to create > a common header file that can contain these forward declarations. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > platform/linux-generic/Makefile.am | 1 + > .../linux-generic/include/odp_buffer_internal.h | 11 ++------ > .../include/odp_forward_typedefs_internal.h | 32 ++++++++++++++++++++++ > .../linux-generic/include/odp_queue_internal.h | 5 ++-- > 4 files changed, 39 insertions(+), 10 deletions(-) > create mode 100644 platform/linux-generic/include/odp_forward_typedefs_internal.h > > diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am > index f2c081a..4c79730 100644 > --- a/platform/linux-generic/Makefile.am > +++ b/platform/linux-generic/Makefile.am > @@ -122,6 +122,7 @@ noinst_HEADERS = \ > ${srcdir}/include/odp_classification_internal.h \ > ${srcdir}/include/odp_crypto_internal.h \ > ${srcdir}/include/odp_debug_internal.h \ > + ${srcdir}/include/odp_forward_typedefs_internal.h \ > ${srcdir}/include/odp_internal.h \ > ${srcdir}/include/odp_packet_internal.h \ > ${srcdir}/include/odp_packet_io_internal.h \ > diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h > index 6badeba..4cacca1 100644 > --- a/platform/linux-generic/include/odp_buffer_internal.h > +++ b/platform/linux-generic/include/odp_buffer_internal.h > @@ -29,7 +29,7 @@ extern "C" { > #include <odp/byteorder.h> > #include <odp/thread.h> > #include <odp/event.h> > - > +#include <odp_forward_typedefs_internal.h> > > #define ODP_BITSIZE(x) \ > ((x) <= 2 ? 1 : \ > @@ -101,13 +101,8 @@ typedef union odp_buffer_bits_t { > }; > } odp_buffer_bits_t; > > -/* forward declaration */ > -struct odp_buffer_hdr_t; > -union queue_entry_u; > -typedef union queue_entry_u queue_entry_t; > - > /* Common buffer header */ > -typedef struct odp_buffer_hdr_t { > +struct odp_buffer_hdr_t { > struct odp_buffer_hdr_t *next; /* next buf in a list--keep 1st */ > union { /* Multi-use secondary link */ > struct odp_buffer_hdr_t *prev; > @@ -144,7 +139,7 @@ typedef struct odp_buffer_hdr_t { > queue_entry_t *target_qe; /* ordered queue target */ > uint64_t sync; /* for ordered synchronization */ > }; > -} odp_buffer_hdr_t; > +}; > > /** @internal Compile time assert that the > * allocator field can handle any allocator id*/ > diff --git a/platform/linux-generic/include/odp_forward_typedefs_internal.h b/platform/linux-generic/include/odp_forward_typedefs_internal.h > new file mode 100644 > index 0000000..f8832f7 > --- /dev/null > +++ b/platform/linux-generic/include/odp_forward_typedefs_internal.h > @@ -0,0 +1,32 @@ > +/* Copyright (c) 2015, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP forward typedefs - implementation internal > + * > + * This needs to be a separate file because it is needed by both > + * odp_queue_internal.h and odp_buffer_internal.h and clang prohibits forward > + * "redefining" typedefs. Note that this file can be extended with additional > + * forward typedefs as needed. > + */ > + > +#ifndef ODP_FORWARD_TYPEDEFS_INTERNAL_H_ > +#define ODP_FORWARD_TYPEDEFS_INTERNAL_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +typedef struct odp_buffer_hdr_t odp_buffer_hdr_t; > +typedef union queue_entry_u queue_entry_t; > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > diff --git a/platform/linux-generic/include/odp_queue_internal.h b/platform/linux-generic/include/odp_queue_internal.h > index 0f30965..19a0f07 100644 > --- a/platform/linux-generic/include/odp_queue_internal.h > +++ b/platform/linux-generic/include/odp_queue_internal.h > @@ -19,6 +19,7 @@ extern "C" { > #endif > > #include <odp/queue.h> > +#include <odp_forward_typedefs_internal.h> > #include <odp_buffer_internal.h> > #include <odp_align_internal.h> > #include <odp/packet_io.h> > @@ -86,10 +87,10 @@ struct queue_entry_s { > odp_atomic_u64_t sync_out; > }; > > -typedef union queue_entry_u { > +union queue_entry_u { > struct queue_entry_s s; > uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(struct queue_entry_s))]; > -} queue_entry_t; > +}; > > > queue_entry_t *get_qentry(uint32_t queue_id);
diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index f2c081a..4c79730 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -122,6 +122,7 @@ noinst_HEADERS = \ ${srcdir}/include/odp_classification_internal.h \ ${srcdir}/include/odp_crypto_internal.h \ ${srcdir}/include/odp_debug_internal.h \ + ${srcdir}/include/odp_forward_typedefs_internal.h \ ${srcdir}/include/odp_internal.h \ ${srcdir}/include/odp_packet_internal.h \ ${srcdir}/include/odp_packet_io_internal.h \ diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h index 6badeba..4cacca1 100644 --- a/platform/linux-generic/include/odp_buffer_internal.h +++ b/platform/linux-generic/include/odp_buffer_internal.h @@ -29,7 +29,7 @@ extern "C" { #include <odp/byteorder.h> #include <odp/thread.h> #include <odp/event.h> - +#include <odp_forward_typedefs_internal.h> #define ODP_BITSIZE(x) \ ((x) <= 2 ? 1 : \ @@ -101,13 +101,8 @@ typedef union odp_buffer_bits_t { }; } odp_buffer_bits_t; -/* forward declaration */ -struct odp_buffer_hdr_t; -union queue_entry_u; -typedef union queue_entry_u queue_entry_t; - /* Common buffer header */ -typedef struct odp_buffer_hdr_t { +struct odp_buffer_hdr_t { struct odp_buffer_hdr_t *next; /* next buf in a list--keep 1st */ union { /* Multi-use secondary link */ struct odp_buffer_hdr_t *prev; @@ -144,7 +139,7 @@ typedef struct odp_buffer_hdr_t { queue_entry_t *target_qe; /* ordered queue target */ uint64_t sync; /* for ordered synchronization */ }; -} odp_buffer_hdr_t; +}; /** @internal Compile time assert that the * allocator field can handle any allocator id*/ diff --git a/platform/linux-generic/include/odp_forward_typedefs_internal.h b/platform/linux-generic/include/odp_forward_typedefs_internal.h new file mode 100644 index 0000000..f8832f7 --- /dev/null +++ b/platform/linux-generic/include/odp_forward_typedefs_internal.h @@ -0,0 +1,32 @@ +/* Copyright (c) 2015, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * ODP forward typedefs - implementation internal + * + * This needs to be a separate file because it is needed by both + * odp_queue_internal.h and odp_buffer_internal.h and clang prohibits forward + * "redefining" typedefs. Note that this file can be extended with additional + * forward typedefs as needed. + */ + +#ifndef ODP_FORWARD_TYPEDEFS_INTERNAL_H_ +#define ODP_FORWARD_TYPEDEFS_INTERNAL_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct odp_buffer_hdr_t odp_buffer_hdr_t; +typedef union queue_entry_u queue_entry_t; + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/platform/linux-generic/include/odp_queue_internal.h b/platform/linux-generic/include/odp_queue_internal.h index 0f30965..19a0f07 100644 --- a/platform/linux-generic/include/odp_queue_internal.h +++ b/platform/linux-generic/include/odp_queue_internal.h @@ -19,6 +19,7 @@ extern "C" { #endif #include <odp/queue.h> +#include <odp_forward_typedefs_internal.h> #include <odp_buffer_internal.h> #include <odp_align_internal.h> #include <odp/packet_io.h> @@ -86,10 +87,10 @@ struct queue_entry_s { odp_atomic_u64_t sync_out; }; -typedef union queue_entry_u { +union queue_entry_u { struct queue_entry_s s; uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(struct queue_entry_s))]; -} queue_entry_t; +}; queue_entry_t *get_qentry(uint32_t queue_id);
With the addition of ordered queues, there is a circular typedef relationship between odp_queue_internal.h and odp_buffer_internal.h. The standard forward declaration technique that GCC accepts is strictly not acceptable to C99 and is flagged by clang. The solution is to create a common header file that can contain these forward declarations. Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- platform/linux-generic/Makefile.am | 1 + .../linux-generic/include/odp_buffer_internal.h | 11 ++------ .../include/odp_forward_typedefs_internal.h | 32 ++++++++++++++++++++++ .../linux-generic/include/odp_queue_internal.h | 5 ++-- 4 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 platform/linux-generic/include/odp_forward_typedefs_internal.h