[dpdk-dev] [PATCH v2 01/12] eal: define container_of macro

[Adrien Mazarguil]

On Fri, Dec 16, 2016 at 11:47:14AM +0100, Jan Blunck wrote:
> On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
> <adrien.mazarguil at 6wind.com> wrote:
> > On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
> >> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
> >> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
> >> >>
> >> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain at nxp.com>
> >> >> wrote:
> >> >>>
> >> >>> From: Jan Blunck <jblunck at infradead.org>
> >> >>>
> >> >>> This macro is based on Jan Viktorin's original patch but also checks the
> >> >>> type of the passed pointer against the type of the member.
> >> >>>
> >> >>> Signed-off-by: Jan Viktorin <viktorin at rehivetech.com>
> >> >>> [shreyansh.jain at nxp.com: Fix checkpatch error]
> >> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>
> >> >>> [jblunck at infradead.org: add type checking and __extension__]
> >> >>> Signed-off-by: Jan Blunck <jblunck at infradead.org>
> >> >>>
> >> >>> --
> >> >>> v2:
> >> >>>  - fix checkpatch error
> >> >>> ---
> >> >>>  lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
> >> >>>  1 file changed, 21 insertions(+)
> >> >>>
> >> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
> >> >>> b/lib/librte_eal/common/include/rte_common.h
> >> >>> index db5ac91..3eb8d11 100644
> >> >>> --- a/lib/librte_eal/common/include/rte_common.h
> >> >>> +++ b/lib/librte_eal/common/include/rte_common.h
> >> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
> >> >>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
> >> >>>  #endif
> >> >>>
> >> >>> +/**
> >> >>> + * Return pointer to the wrapping struct instance.
> >> >>> + *
> >> >>> + * Example:
> >> >>> + *
> >> >>> + *  struct wrapper {
> >> >>> + *      ...
> >> >>> + *      struct child c;
> >> >>> + *      ...
> >> >>> + *  };
> >> >>> + *
> >> >>> + *  struct child *x = obtain(...);
> >> >>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
> >> >>> + */
> >> >>> +#ifndef container_of
> >> >>> +#define container_of(ptr, type, member)        (__extension__  ({
> >> >>> \
> >> >>> +                       typeof(((type *)0)->member) * _ptr = (ptr);     \
> >> >>> +                       (type *)(((char *)_ptr) - offsetof(type,
> >> >>> member));\
> >> >>> +                       }))
> >> >>
> >> >>
> >> >> This is a checkpatch false positive. It should be fine to ignore this.
> >> >> IIRC we already discussed this before.
> >> >
> >> >
> >> > I too thought something similar was discussed. I tried searching the
> >> > archives but couldn't find anything - thus, I thought probably I was
> >> > hallucinating :P
> >> >
> >> > So, you want me to revert back the '()' change? Does it impact the expansion
> >> > of this macro?
> >>
> >> We haven't added this on any other usage of the __extension__ keyword
> >> in the existing code. From my perspective it is more consistent to
> >> revert it.
> >>
> >> Anyone else with an opinion here? David? Thomas?
> >
> > As an exported header, rte_common.h must pass check-includes.sh. Both
> > typeof() and the ({ ... }) construct are non-standard GCC extensions and
> > would fail to compile with pedantic options.
> >
> 
> Thanks Adrien. These extensions are already in use by rte_common.h and
> other headers. I don't believe we can remove the usage of typeof()
> that easily without making the code really ugly.

Sure, no problem with that, I misread and thought you wanted to drop
__extension__ as well.

Parentheses may perhaps cause more accurate warnings in case of syntax
errors. That is not significant so either way is fine by me.

-- 
Adrien Mazarguil
6WIND