On Thu, 28 Aug 2025 at 16:08, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> ... from ->mnt_flags to LSB of ->mnt_pprev_for_sb.
Ugh. This one I'm not happy with.
The random new casts:
> static inline void mnt_del_instance(struct mount *m)
> {
> - struct mount **p = m->mnt_pprev_for_sb;
> + struct mount **p = (void *)m->mnt_pprev_for_sb;
> struct mount *next = m->mnt_next_for_sb;
>
> if (next)
> - next->mnt_pprev_for_sb = p;
> + next->mnt_pprev_for_sb = (unsigned long)p;
> *p = next;
> }
are just nasty. And it's there in multiple places (ie
mnt_add_instance() has more of them).
Making things even *worse*, the other case you changed (s_mounts) it's
a "void *", which means that it does *not* have casts in other places,
and you still do things like
for (struct mount *m = sb->s_mounts; m; m = m->mnt_next_for_sb) {
so that 's_mounts' thing is just silently cast from a untyped 'void *'
to the 'struct mount *' that it used to be.
So no - this is *not* acceptable.
Same largely goes for that
> - struct mount **mnt_pprev_for_sb;/* except that LSB of pprev will be stolen */
> + unsigned long mnt_pprev_for_sb; /* except that LSB of pprev is stolen */
change, but at least there it's now a 'unsigned long', so it will
*always* complain if a cast is missing in either direction. That's
better, but still horrendously ugly.
If you want to use an opaque type, then please make it be truly
opaque. Not 'unsigned long'. And certainly not 'void *'. Make it be
something that is still type-safe - you can make up a pointer to
struct name that is never actually declared, so that it's basically a
unique type (or two separate types for mnt_pprev_for_sb and
I'm not even clear on why you did this change, but if you want to have
specific types for some reason, make them *really* specific. Don't
make them 'void *', and 'unsigned long'.
Linus
