[dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file

[dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file

[Bruce Richardson]

The ENQUEUE_PTRS and DEQUEUE_PTRS macros defined in rte_ring.h are
not meant to be global and are not prefixed with the RTE_ prefix.
Therefore undef the macros at end of file to avoid pollution of the
global namespace, in case ends apps end up wanting to reuse those names.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_ring/rte_ring.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index bdf69b7..0d35648 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -1232,6 +1232,10 @@ rte_ring_dequeue_burst(struct rte_ring *r, void **obj_table, unsigned n)
 		return rte_ring_mc_dequeue_burst(r, obj_table, n);
 }
 
+/* undef un-prefixed macros which are local to this file */
+#undef ENQUEUE_PTRS
+#undef DEQUEUE_PTRS
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.1.0

Re: [dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file

[Thomas Monjalon]

2015-03-03 16:38, Bruce Richardson:
> The ENQUEUE_PTRS and DEQUEUE_PTRS macros defined in rte_ring.h are
> not meant to be global and are not prefixed with the RTE_ prefix.
> Therefore undef the macros at end of file to avoid pollution of the
> global namespace, in case ends apps end up wanting to reuse those names.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_ring/rte_ring.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> index bdf69b7..0d35648 100644
> --- a/lib/librte_ring/rte_ring.h
> +++ b/lib/librte_ring/rte_ring.h
> @@ -1232,6 +1232,10 @@ rte_ring_dequeue_burst(struct rte_ring *r, void **obj_table, unsigned n)
>  		return rte_ring_mc_dequeue_burst(r, obj_table, n);
>  }
>  
> +/* undef un-prefixed macros which are local to this file */
> +#undef ENQUEUE_PTRS
> +#undef DEQUEUE_PTRS
> +

Thanks for trying to clean-up things.
Note that if an application is using this macro name, it will be destroyed
when including rte_ring.h.
Globally, DPDK namespace is awful and I hope we will be able to improve it.

Re: [dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file

[Bruce Richardson]

On Tue, Mar 03, 2015 at 10:03:45PM +0100, Thomas Monjalon wrote:
> 2015-03-03 16:38, Bruce Richardson:
> > The ENQUEUE_PTRS and DEQUEUE_PTRS macros defined in rte_ring.h are
> > not meant to be global and are not prefixed with the RTE_ prefix.
> > Therefore undef the macros at end of file to avoid pollution of the
> > global namespace, in case ends apps end up wanting to reuse those names.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/librte_ring/rte_ring.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> > index bdf69b7..0d35648 100644
> > --- a/lib/librte_ring/rte_ring.h
> > +++ b/lib/librte_ring/rte_ring.h
> > @@ -1232,6 +1232,10 @@ rte_ring_dequeue_burst(struct rte_ring *r, void **obj_table, unsigned n)
> >  		return rte_ring_mc_dequeue_burst(r, obj_table, n);
> >  }
> >  
> > +/* undef un-prefixed macros which are local to this file */
> > +#undef ENQUEUE_PTRS
> > +#undef DEQUEUE_PTRS
> > +
> 
> Thanks for trying to clean-up things.
> Note that if an application is using this macro name, it will be destroyed
> when including rte_ring.h.
> Globally, DPDK namespace is awful and I hope we will be able to improve it.
> 
Only if they are defining such a macro before including rte_ring.h, which I would
expect to be an edge case. Also, in such a case, the compiler/preprocessor will
give an error at the duplicate macro definition stage, and the simple fix is to
reorder the header file inclusion to avoid problems i.e. no changing of dpdk 
required.
I suppose a better fix to go along with this is to RTE-prefix the macros. I'll 
see about doing a V2.

/Bruce

Re: [dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Wednesday, March 04, 2015 10:23 AM
> To: Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file
> 
> On Tue, Mar 03, 2015 at 10:03:45PM +0100, Thomas Monjalon wrote:
> > 2015-03-03 16:38, Bruce Richardson:
> > > The ENQUEUE_PTRS and DEQUEUE_PTRS macros defined in rte_ring.h are
> > > not meant to be global and are not prefixed with the RTE_ prefix.
> > > Therefore undef the macros at end of file to avoid pollution of the
> > > global namespace, in case ends apps end up wanting to reuse those names.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  lib/librte_ring/rte_ring.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> > > index bdf69b7..0d35648 100644
> > > --- a/lib/librte_ring/rte_ring.h
> > > +++ b/lib/librte_ring/rte_ring.h
> > > @@ -1232,6 +1232,10 @@ rte_ring_dequeue_burst(struct rte_ring *r, void **obj_table, unsigned n)
> > >  		return rte_ring_mc_dequeue_burst(r, obj_table, n);
> > >  }
> > >
> > > +/* undef un-prefixed macros which are local to this file */
> > > +#undef ENQUEUE_PTRS
> > > +#undef DEQUEUE_PTRS
> > > +
> >
> > Thanks for trying to clean-up things.
> > Note that if an application is using this macro name, it will be destroyed
> > when including rte_ring.h.
> > Globally, DPDK namespace is awful and I hope we will be able to improve it.
> >
> Only if they are defining such a macro before including rte_ring.h, which I would
> expect to be an edge case. Also, in such a case, the compiler/preprocessor will
> give an error at the duplicate macro definition stage, and the simple fix is to
> reorder the header file inclusion to avoid problems i.e. no changing of dpdk
> required.
> I suppose a better fix to go along with this is to RTE-prefix the macros. I'll
> see about doing a V2.

Why not just to rename it to __RTE_RING_ENQUEUE_PTRS__ or something,
and put in the comments that it is for internal usage?
Konstantin
> 
> /Bruce

Re: [dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file

[Bruce Richardson]

On Wed, Mar 04, 2015 at 10:26:24AM +0000, Ananyev, Konstantin wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Wednesday, March 04, 2015 10:23 AM
> > To: Thomas Monjalon
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ring: cleanup file-local macros at end-of-file
> > 
> > On Tue, Mar 03, 2015 at 10:03:45PM +0100, Thomas Monjalon wrote:
> > > 2015-03-03 16:38, Bruce Richardson:
> > > > The ENQUEUE_PTRS and DEQUEUE_PTRS macros defined in rte_ring.h are
> > > > not meant to be global and are not prefixed with the RTE_ prefix.
> > > > Therefore undef the macros at end of file to avoid pollution of the
> > > > global namespace, in case ends apps end up wanting to reuse those names.
> > > >
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > ---
> > > >  lib/librte_ring/rte_ring.h | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> > > > index bdf69b7..0d35648 100644
> > > > --- a/lib/librte_ring/rte_ring.h
> > > > +++ b/lib/librte_ring/rte_ring.h
> > > > @@ -1232,6 +1232,10 @@ rte_ring_dequeue_burst(struct rte_ring *r, void **obj_table, unsigned n)
> > > >  		return rte_ring_mc_dequeue_burst(r, obj_table, n);
> > > >  }
> > > >
> > > > +/* undef un-prefixed macros which are local to this file */
> > > > +#undef ENQUEUE_PTRS
> > > > +#undef DEQUEUE_PTRS
> > > > +
> > >
> > > Thanks for trying to clean-up things.
> > > Note that if an application is using this macro name, it will be destroyed
> > > when including rte_ring.h.
> > > Globally, DPDK namespace is awful and I hope we will be able to improve it.
> > >
> > Only if they are defining such a macro before including rte_ring.h, which I would
> > expect to be an edge case. Also, in such a case, the compiler/preprocessor will
> > give an error at the duplicate macro definition stage, and the simple fix is to
> > reorder the header file inclusion to avoid problems i.e. no changing of dpdk
> > required.
> > I suppose a better fix to go along with this is to RTE-prefix the macros. I'll
> > see about doing a V2.
> 
> Why not just to rename it to __RTE_RING_ENQUEUE_PTRS__ or something,
> and put in the comments that it is for internal usage?
> Konstantin

Yes, I'm planning on renaming it. But since it's for internal use only, I might
as well undef it at the end of the file too. I think it's good practice.

/Bruce