On Wed, Jan 29, 2025 at 05:06:54PM +0300, Dan Carpenter wrote: > On Wed, Jan 29, 2025 at 02:14:14PM +0100, Greg KH wrote: > > 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. > > > > I guess my use case is different from Gustavo's. For him, using > container_of() is fine. We probably don't even need an assert because > once you see a struct_group_tagged() then you know the order is important.
Who knows this? The developer? What are they supposed to "know" here? I sure don't :) Having some sort of "__must_be_first" marking for a field is fine and break the build if that doesn't happen. Otherwise this is something that is not going to be used properly over time. > For me, it's code like I mentioned which does: > > p = container_of(); > if (IS_ERR(p)) > ... > > And I did see you suggest that people re-write that kind of code, but no > one is going to do that. :P People know that container_of() is just a > cast in that case. It works fine. It's just a bit ugly. It doesn't work if the field isn't first, so no, it shouldn't be working fine, and that should be flagged and fixed and never allowed to come back again. Can't we do that with coccinelle? thanks, greg k-h
