The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Vincent Mailhol <mailhol@kernel.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Alexander Lobakin <alobakin@pm.me>, Arnd Bergmann <arnd@arndb.de>,
	David Sterba <dsterba@suse.cz>, Ivo van Doorn <ivdoorn@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] container_of: remove useless pair of parentheses
Date: Thu, 16 Jul 2026 16:09:37 +0200	[thread overview]
Message-ID: <2026071624-lunar-casually-55fb@gregkh> (raw)
In-Reply-To: <e386a121-d304-418a-9bab-9de5716a99e0@kernel.org>

On Wed, Jul 15, 2026 at 06:27:31PM +0200, Vincent Mailhol wrote:
> On 15/07/2026 at 16:57, Greg Kroah-Hartman wrote:
> > On Wed, Jul 15, 2026 at 04:51:28PM +0200, Vincent Mailhol wrote:
> >> On 15/07/2026 at 16:30, Alexey Dobriyan wrote:
> >>> On Tue, Jul 14, 2026 at 08:18:02PM +0200, Vincent Mailhol wrote:
> >>>> The last expression in container_of() doesn't need an extra pair of
> >>>> parenthesis. Remove it.
> >>>>
> >>>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> >>>> ---
> >>>>  include/linux/container_of.h | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> >>>> index 28500a62ab7e..68153170db32 100644
> >>>> --- a/include/linux/container_of.h
> >>>> +++ b/include/linux/container_of.h
> >>>> @@ -21,7 +21,7 @@
> >>>>  	static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
> >>>>  		      __same_type(*(ptr), void),			\
> >>>>  		      "pointer type mismatch in container_of()");	\
> >>>> -	((type *)(__mptr - offsetof(type, member))); })
> >>>> +	(type *)(__mptr - offsetof(type, member)); })
> >>>
> >>> container_of_const() has the same problem.
> >>
> >> Yes. But scoped I my series to the container_of() macro only.
> >>
> >> I don't plan to send a follow-up patch to remove the other useless
> >> parenthesis. The things which really bothered me was the __mptr
> >> variable which I removed in patch #3. And with that, I am done with
> >> what I wanted to contribute in this file.
> >>
> >> But you are welcome to do this clean-up if you feel the need for it.
> >>
> >>> And it should parenthesize first argument to _Generic.
> >>
> >> Do you mean like this:
> >>
> >> 	#define container_of_const(ptr, type, member)				\
> >> 		_Generic((ptr),							\
> >> 			const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
> >> 			default: ((type *)container_of(ptr, type, member))	\
> >> 		)
> >>
> >> ?
> >>
> >> Why would parenthesis be needed here? I don't see what kind of
> >> expression would make the current implementation unsafe.
> > 
> > It took me a while working with container_of_const() to get it right
> > with the (), so be careful removing any.
> 
> I just tested this patch:
> 
> ---8<---
> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> index 1f6ebf27d962..03d5b91534a7 100644
> --- a/include/linux/container_of.h
> +++ b/include/linux/container_of.h
> @@ -34,8 +34,8 @@
>   */
>  #define container_of_const(ptr, type, member)				\
>  	_Generic(ptr,							\
> -		const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
> -		default: ((type *)container_of(ptr, type, member))	\
> +		const typeof(*(ptr)) *: (const type *)container_of(ptr, type, member),\
> +		default: (type *)container_of(ptr, type, member)	\
>  	)
>   #endif	/* _LINUX_CONTAINER_OF_H */
> ---8<---
> 
> I could compile and boot it, no problem (I am actually writing this
> message from a machine on which this patch is applied).
> 
> So, yes, I believe that these two pairs of parenthesis can be removed.
> Just not sure if it is worth it. For container_of(), the parenthesis
> removal was part of a series with more meaningful changes. I don't
> feel exited of sending another series to just remove the
> container_of_const() parenthesis.

Yeah, if it's not hurting anything, let's just leave it as-is for now.

thanks,

greg k-h

  reply	other threads:[~2026-07-16 14:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14 18:18 [PATCH 0/3] container_of: refactors Vincent Mailhol
2026-07-14 18:18 ` [PATCH 1/3] container_of: apply typeof_member() to container_of() Vincent Mailhol
2026-07-14 18:18 ` [PATCH 2/3] container_of: remove useless pair of parentheses Vincent Mailhol
2026-07-15 14:30   ` Alexey Dobriyan
2026-07-15 14:51     ` Vincent Mailhol
2026-07-15 14:57       ` Greg Kroah-Hartman
2026-07-15 16:27         ` Vincent Mailhol
2026-07-16 14:09           ` Greg Kroah-Hartman [this message]
2026-07-14 18:18 ` [PATCH 3/3] container_of: remove local __mptr variable Vincent Mailhol
2026-07-15  4:54 ` [PATCH 0/3] container_of: refactors Greg Kroah-Hartman
2026-07-15  5:33   ` Vincent Mailhol

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026071624-lunar-casually-55fb@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=alobakin@pm.me \
    --cc=arnd@arndb.de \
    --cc=dsterba@suse.cz \
    --cc=ivdoorn@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mailhol@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox