Messages in this thread | | | Date | Wed, 7 Dec 2022 10:26:54 +0100 | Subject | Re: [PATCH v2 1/4] container_of: add container_of_const() that preserves const-ness of the pointer | From | Rasmus Villemoes <> |
| |
On 06/12/2022 21.18, Matthew Wilcox wrote: > On Tue, Dec 06, 2022 at 07:46:47PM +0100, Greg Kroah-Hartman wrote: >> On Tue, Dec 06, 2022 at 05:18:22PM +0000, Matthew Wilcox wrote: >>> static inline struct external_name *external_name(struct dentry *dentry) >>> { >>> - return container_of(dentry->d_name.name, struct external_name, name[0]); >>> + return container_of_not_const(dentry->d_name.name, >>> + struct external_name, name[0]); >>> } >> >> Will just: >> return container_of((unsigned char *)dentry->d_name.name, struct external_name, name[0]); >> work by casting away the "const" of the name? >> >> Yeah it's ugly, I never considered the address of a const char * being >> used as a base to cast back from. The vfs is fun :) > > Yes, that also works. This isn't particularly common in the VFS, it's > just the dcache. And I understand why it's done like this; you don't > want rando filesystems modifying dentry names without also updating > the hash. > > I feel like all the options here are kind of ugly. Seeing casts in > the arguments to container_of should be a red flag! > > Here's a bit of a weird option ... > > +#define container_of_2(ptr, p_m, type, member) \ > + _Generic(ptr, \ > + const typeof(*(ptr)) *: (const type *)container_of(ptr->p_m, type, member), \ > + default: ((type *)container_of(ptr->p_m, type, member))) > + > > static inline struct external_name *external_name(struct dentry *dentry) > { > - return container_of(dentry->d_name.name, struct external_name, name[0]); > + return container_of_2(dentry, d_name.name, struct external_name, > + name[0]); > } > > so we actually split the first argument into two -- the pointer which > isn't const, then the pointer member which might be const, but we don't > use it for the return result of container_of_2.
Wait, what? The const-ness or not of dentry is completely irrelevant, we're not doing any pointer arithmetic on that to obtain some other pointer that morally should have the same const-ness. We're dereferencing dentry to get a pointer value, and _that_ pointer value is then subject to the pointer arithmetic.
Note that external_name might as well have had the prototype
static inline struct external_name *external_name(const struct dentry *dentry)
and then your container_of_2 would break.
I think that if we want to move towards container_of preserving the constness of the member pointer, the right fix here is indeed a cast, but not inside container_of, but rather to cast away the const afterwards:
return (struct external_name *)container_of(dentry->d_name.name, struct external_name, name[0]);
knowing that yes, the dentry only keeps a const pointer to the name[] member for good reasons, but the callers very much do need to modify the rest of the struct.
Rasmus
| |