Re: [PATCH v2][next] container_of: add container_first() macro

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Jan 29, 2025 at 01:39:27PM +0300, Dan Carpenter wrote:
> On Wed, Jan 29, 2025 at 09:34:07AM +0100, Greg KH wrote:
> > On Wed, Jan 29, 2025 at 06:35:18PM +1030, Gustavo A. R. Silva wrote:
> > > 
> > > 
> > > On 29/01/25 16:24, Greg KH wrote:
> > > > On Wed, Jan 29, 2025 at 03:56:01PM +1030, Gustavo A. R. Silva wrote:
> > > > > This is like container_of_const() but it contains an assert to
> > > > > ensure that it's using the first member in the structure.
> > > > 
> > > > But why?  If you "know" it's the first member, just do a normal cast.
> > > > If you don't, then you probably shouldn't be caring about this anyway,
> > > > right?
> > > 
> > > This is more about the cases where the member _must_ be first in the
> > > structure. See below for an example related to -Wflex-array-member-not-at-end
> > 
> > That's fine, but that's a build-time issue, you should enforce that in
> > the structure itself, why are you forcing people to remember to use this
> > macro when you want to use the field?  There's nothing preventing anyone
> > from using container_of() instead here, and nothing will catch that from
> > what I can tell.
> 
> The new definition has a static_assert() in it so it's enforced about
> build time.

Yes, but that forces you to "know" to do that in the .c file.  How do
you know to use this, and if you remove it or change it to
container_of(), it works just fine again.

> +#define container_first(ptr, type, member) ({                           \
> +       static_assert(offsetof(type, member) == 0, "not first member"); \
> +       container_of_const(ptr, type, member); })
> 
> That was the discussion at plumbers, Gustavo just wanted to use
> container_of() but I told him I was tired of code which assumes that
> container_of() is just a cast.  If we're going to write code with that
> assumption then lets create a different macro for it and let's make the
> build break if someone changes it.

That's fine, but it should be where the variable layout is, NOT where
you dereference the pointer as at that point in time, you don't know or
care if the location is in the first location at all.

thanks,

greg k-h