Re: [PATCH 0029/1285] Replace numeric parameter like 0444 with macro
From: Linus Torvalds
Date: Tue Aug 02 2016 - 13:29:22 EST
On Tue, Aug 2, 2016 at 8:16 AM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>> }
>> -static DEVICE_ATTR(chip_width, 0444, chip_width_show, NULL);
>> +static DEVICE_ATTR(chip_width, S_IRUSR | S_IRGRP | S_IROTH, chip_width_show, NULL);
I detest these patches.
It's actually *MUCH* easer to see that "oh, everybody gets to read"
when you see something like 0444, and seeing 0600 is a very clear
"owner only".
I contrast, something like S_IRUSR | S_IRGRP | S_IROTH you actually
have to _read_ to figure it out.
I seriously believe that when it comes to UNIX chmod masks, the octal
representation is much better.
> You do know about S_IRUGO, right? Why not use that?
Actually, that one is even worse. And part of it is exactly that there
are nineteen billion of these "random letter combinations" that are
visually almost entirely indistinguishable from each other, so part of
the problem with the symbolic names is exactly that you have to read
them, and you have to read them *CAREFULLY*.
In contrast, 0444 is completely unambiguous, and doesn't take a lot of
effort to parse.
Honestly, the whole "use symbolic names instead of numbers" only makes
sense if the symbolic names are easier to parse or understand. In the
case of the unix permission masks, that simply isn't the case. Never
has been. Those names are an abomination, and should just die.
So I don't want to see _any_ of these patches being applied,
personally. Much less being bombarded with hundreds of patches with
the same subject line and contents.
Linus