RE: [PATCH 1/1] linux/container_of.h: Warn about loss of constness

From: David Laight <David.Laight@ACULAB.COM>
To: 'Andy Shevchenko' <andriy.shevchenko@linux.intel.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Subject: RE: [PATCH 1/1] linux/container_of.h: Warn about loss of constness
Date: Mon, 24 Oct 2022 09:34:42 +0000	[thread overview]
Message-ID: <fbe735d131a44c2a95248cc4ad51485b@AcuMS.aculab.com> (raw)
In-Reply-To: <Y1ZZcL/Q7QJ+YYhJ@smile.fi.intel.com>

From: Andy Shevchenko
> Sent: 24 October 2022 10:23
...
> > > Wait, no one uses this macro, so why not just remove it entirely?
> >
> > Good question. It appears to be a (relatively) common pattern to look up
> > something and the return its containing object if the lookup was
> > successful. Doing a quick
> >
> > 	$ git grep 'container_of.*:' drivers include
> >
> > reveals more than 20 instances of the pattern. There are probably more
> > those that use if for testing for NULL. I guess people don't know about
> > this macro, apart from the developers of the staging driver it was added
> > for (commit 05e6557b8ed833546ee2b66ce6b58fecf09f439e).
> 
> Maybe we can provide an example to keep this macro in the kernel, meaning
> convert one of the drivers / subsystem to actually use it?

Adding _safe() to a function name doesn't actually tell you anything.
You still need to look up what it is 'safe' against.

In this case the full code pattern is actually much clearer.

It is also quite likely that it is followed by an:
	if (!ptr)
		return xxx;
You that can/should really be put before the container_of() call.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)