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

Shreyansh Jain shreyansh.jain at nxp.com
Fri Dec 16 12:54:02 CET 2016


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 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.
>

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


More information about the dev mailing list