On Mon, Jun 20, 2022 at 8:50 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> Introduces new kmnt{g,u}id_t types. Similar to k{g,u}id_t the new types
> are just simple wrapper structs around regular {g,u}id_t types.
Thanks for working on this.,
I haven't actually perused the series yet, but wanted to just
immediately react to "please don't make random-letter type names".
"gid" is something people understand. It's a thing.
"kgid" kind of made sense, in that it's the "kernel view of the gid",
and it was still short and fairly legible.
"kmntgid" is neither short, legible, or makes sense.
For one thing, the "k" part no longer makes any sense. It's not about
the "kernel view of the gid" any more. Sure, it's "kernel" in the
sense that any kernel code is "kernel", but it's no longer some kind
of "unified kernel view of a user-namespace gid".
So the "k" in "kgid" doesn't make sense because it's a kernel thing,
but more as a negative: "it is *not* the user visible gid".
So instead of changing all our old "git_t" definitions to be "ugid_t"
(for "user visible gid") the "kgid_t" thing was done.
As a result: when you translate it to the mount namespace, I do not
believe that the "k" makes sense any more, because now the point to
distinguish it from "user gids" no longer exists. So it's just one
random letter. In a long jumble of letters that isn't really very
legible or pronounceable.
If it didn't have that 'i' in it, I would think it's a IBM mnemonic
(and I use the word "mnemonic" ironically) for some random assembler
instruction. They used up all the vowels they were willing to use for
the "eieio" instructions, and all other instruction names are a jumble
of random consonants.
So please try to make the type names less of a random jumble of
letters picked from a bag.
That "kmnt[gu]id" pattern exists elsewhere too, in the conversion
functions etc, so it's not just the type name, but more of a generic
"please don't use letter-jumble names".
Maybe just "mnt_[gu]id"" instead of "kmnt[gu]id" would be better.
But even that smells wrong to me. Isn't it really "the guid/uid seen
by the filesystem after the mount mapping"? Wouldn't it be nice to
name by the same "seen by users" and "seen by kernel" to be "seen by
filesystrem"? So wouln't a name like "fs_[gu]id_t" make even more
sense?
I dunno. Maybe I'm thinking about this the wrong way, but I wish the
names would be more explanatory. My personal mental image is that the
user namespaces map a traditional uid into the "kernel id space", and
then the mount id mappings map into the "filesystem id space". Which
is why to me that "k" doesn't make sense, and the "mnt" doesn't really
make tons of sense either (the mount is the thing that _maps_ the id
spaces, but it's not the end result of said mapping).
IOW, I get the feeling that you've named the result with the mapping,
not with the end use. That would be like naming "kuid" by the mapping
(usernamespace), not the end result (the kernel namespace).
But maybe it's just me that is confused here. Particularly since I
didn't really more than *very* superficially and quickly scan the
patches.
Linus
