From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH v2 01/12] eal: define container_of macro Date: Fri, 16 Dec 2016 17:24:02 +0530 Message-ID: References: <1480846288-2517-1-git-send-email-shreyansh.jain@nxp.com> <1481636232-2300-1-git-send-email-shreyansh.jain@nxp.com> <1481636232-2300-2-git-send-email-shreyansh.jain@nxp.com> <3310c320-fa39-cd8c-ab77-ced20daa5073@nxp.com> <20161216092306.GD10340@6wind.com> <20161216112140.GE10340@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: , David Marchand , Thomas Monjalon , Ferruh Yigit , , Jan Viktorin To: Adrien Mazarguil , Jan Blunck Return-path: Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-by2nam03on0084.outbound.protection.outlook.com [104.47.42.84]) by dpdk.org (Postfix) with ESMTP id 0B02A379E for ; Fri, 16 Dec 2016 12:51:28 +0100 (CET) In-Reply-To: <20161216112140.GE10340@6wind.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >> 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 wrote: >>>>> On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote: >>>>>> >>>>>> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain >>>>>> wrote: >>>>>>> >>>>>>> From: Jan Blunck >>>>>>> >>>>>>> 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 >>>>>>> [shreyansh.jain@nxp.com: Fix checkpatch error] >>>>>>> Signed-off-by: Shreyansh Jain >>>>>>> [jblunck@infradead.org: add type checking and __extension__] >>>>>>> Signed-off-by: Jan Blunck >>>>>>> >>>>>>> -- >>>>>>> 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