From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v2 01/12] eal: define container_of macro Date: Fri, 16 Dec 2016 12:21:40 +0100 Message-ID: <20161216112140.GE10340@6wind.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Shreyansh Jain , dev@dpdk.org, David Marchand , Thomas Monjalon , Ferruh Yigit , jianbo.liu@linaro.org, Jan Viktorin To: Jan Blunck Return-path: Received: from mail-wj0-f175.google.com (mail-wj0-f175.google.com [209.85.210.175]) by dpdk.org (Postfix) with ESMTP id EB44D379E for ; Fri, 16 Dec 2016 12:21:48 +0100 (CET) Received: by mail-wj0-f175.google.com with SMTP id tk12so91066277wjb.3 for ; Fri, 16 Dec 2016 03:21:48 -0800 (PST) Content-Disposition: inline In-Reply-To: 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 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. -- Adrien Mazarguil 6WIND