diff mbox

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

Message ID 1481636232-2300-2-git-send-email-shreyansh.jain@nxp.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Shreyansh Jain Dec. 13, 2016, 1:37 p.m. UTC 
  From: Jan Blunck <jblunck@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@rehivetech.com>
[shreyansh.jain@nxp.com: Fix checkpatch error]
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
[jblunck@infradead.org: add type checking and __extension__]
Signed-off-by: Jan Blunck <jblunck@infradead.org>

--
v2:
 - fix checkpatch error
---
 lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Jan Blunck Dec. 13, 2016, 10:24 p.m. UTC | #1 
On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> From: Jan Blunck <jblunck@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@rehivetech.com>
> [shreyansh.jain@nxp.com: Fix checkpatch error]
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> [jblunck@infradead.org: add type checking and __extension__]
> Signed-off-by: Jan Blunck <jblunck@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.


> +#endif
> +
>  #define _RTE_STR(x) #x
>  /** Take a macro value and get a string version of it */
>  #define RTE_STR(x) _RTE_STR(x)
> --
> 2.7.4
>
Shreyansh Jain Dec. 14, 2016, 5:12 a.m. UTC | #2 
On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> From: Jan Blunck <jblunck@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@rehivetech.com>
>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> [jblunck@infradead.org: add type checking and __extension__]
>> Signed-off-by: Jan Blunck <jblunck@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?

>
>
>> +#endif
>> +
>>  #define _RTE_STR(x) #x
>>  /** Take a macro value and get a string version of it */
>>  #define RTE_STR(x) _RTE_STR(x)
>> --
>> 2.7.4
>>
>
Jan Blunck Dec. 16, 2016, 8:14 a.m. UTC | #3 
On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@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@nxp.com>
>> wrote:
>>>
>>> From: Jan Blunck <jblunck@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@rehivetech.com>
>>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> [jblunck@infradead.org: add type checking and __extension__]
>>> Signed-off-by: Jan Blunck <jblunck@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?


>
>>
>>
>>> +#endif
>>> +
>>>  #define _RTE_STR(x) #x
>>>  /** Take a macro value and get a string version of it */
>>>  #define RTE_STR(x) _RTE_STR(x)
>>> --
>>> 2.7.4
>>>
>>
>
Adrien Mazarguil Dec. 16, 2016, 9:23 a.m. UTC | #4 
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@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@nxp.com>
> >> wrote:
> >>>
> >>> From: Jan Blunck <jblunck@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@rehivetech.com>
> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >>> [jblunck@infradead.org: add type checking and __extension__]
> >>> Signed-off-by: Jan Blunck <jblunck@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.
Jan Blunck Dec. 16, 2016, 10:47 a.m. UTC | #5 
On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
<adrien.mazarguil@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@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@nxp.com>
>> >> wrote:
>> >>>
>> >>> From: Jan Blunck <jblunck@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@rehivetech.com>
>> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> >>> [jblunck@infradead.org: add type checking and __extension__]
>> >>> Signed-off-by: Jan Blunck <jblunck@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.
Adrien Mazarguil Dec. 16, 2016, 11:21 a.m. UTC | #6 
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@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@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@nxp.com>
> >> >> wrote:
> >> >>>
> >> >>> From: Jan Blunck <jblunck@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@rehivetech.com>
> >> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
> >> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >> >>> [jblunck@infradead.org: add type checking and __extension__]
> >> >>> Signed-off-by: Jan Blunck <jblunck@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.
Shreyansh Jain Dec. 16, 2016, 11:54 a.m. UTC | #7 
On Friday 16 December 2016 04:51 PM, Adrien Mazarguil wrote:
> 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@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@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@nxp.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> From: Jan Blunck <jblunck@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@rehivetech.com>
>>>>>>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>>>>>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>>>>>> [jblunck@infradead.org: add type checking and __extension__]
>>>>>>> Signed-off-by: Jan Blunck <jblunck@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.
>

If that is the case, and it is OK to ignore the checkpatche warnings 
because of missing '(' and ')' (or, something else), I too prefer not to 
touch the patch unnecessarily.

I will remove my changes and revert back to original patch as created by 
Jan Blunck.

Thanks for clarifications.

-
Shreyansh
diff mbox

Patch

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));\
+			}))
+#endif
+
 #define _RTE_STR(x) #x
 /** Take a macro value and get a string version of it */
 #define RTE_STR(x) _RTE_STR(x)