File permissions in the kernel
one of the worst patch series submissions in history." In theory, the objective of the patch was reasonable: replace hard-coded constants with their symbolic equivalents. But, it seems, this is a case where the community would rather see the numbers directly.
The change in question relates to places in the kernel where file permissions are specified — usually permissions for files to be created in sysfs or /proc. There is a set of macros defined in <linux/stat.h> for these permissions bits, but it is common practice in the kernel — and among users of Unix-like systems in general — to just use their octal equivalent instead. Thus, for example, one will often see 0444 instead of S_IRUGO. Indeed, it seems one will see it at least 1,285 times, given the length of the patch set sent to eliminate octal permissions from the kernel.
There were obviously lots of complaints about how the patch set was done, but there was also a lot of opposition to the change itself. It seems that many people find a string like 0644 easier to read than S_IWUSR|S_IRUGO. In the end, Linus made that approach official, saying that he did not want to see any of the cleanup patches merged and that, in fact, it would be better to convert users of the macros to octal strings instead.
Octal constants are not perfect either; as Al Viro pointed out, they are subject to subtle and hard-to-see errors. Perhaps, it was suggested, the real problem is that the (POSIX-defined) S_* macros are hard to read, obscuring the developer's intent rather than clarifying it. As an alternative, Ingo Molnar has proposed the adoption of a new set of macros, defined like this:
#define PERM_rw_______ 0600
#define PERM_rw_r_____ 0640
#define PERM_rw_r__r__ 0644
#define PERM_rw_rw_r__ 0664
#define PERM_rw_rw_rw_ 0666
All of the "useful" combinations have macros defined, while there are none
for settings that don't make sense. Use of these macros, he said, would
make the code clearer and make it harder to introduce security problems.
Actually getting them merged, though, might require overcoming the habits
of developers who have been typing octal constants for decades. The
eventual discussion could yet end up being longer than the patch series
that provoked it.
Posted Aug 4, 2016 7:59 UTC (Thu)
by NRArnot (subscriber, #3033)
[Link] (1 responses)
Yuk! How many trailing underscores, and how do they aid comprehension?
I'd suggest PERM_rw_0_0, with 0 standing for no bits set. But I prefer 0600 raw.
Posted Aug 4, 2016 14:42 UTC (Thu)
by mathstuf (subscriber, #69389)
[Link]
Posted Aug 4, 2016 15:25 UTC (Thu)
by epa (subscriber, #39769)
[Link] (15 responses)
Posted Aug 4, 2016 19:51 UTC (Thu)
by k8to (guest, #15413)
[Link] (14 responses)
Posted Aug 5, 2016 5:47 UTC (Fri)
by epa (subscriber, #39769)
[Link] (9 responses)
Posted Aug 5, 2016 6:35 UTC (Fri)
by johill (subscriber, #25196)
[Link] (6 responses)
Posted Aug 5, 2016 6:38 UTC (Fri)
by johill (subscriber, #25196)
[Link] (5 responses)
Posted Aug 5, 2016 6:41 UTC (Fri)
by johill (subscriber, #25196)
[Link] (4 responses)
Posted Aug 5, 2016 19:39 UTC (Fri)
by xtifr (guest, #143)
[Link] (3 responses)
In C, if you put "const int foo = 5;" in a header, and include it in multiple files, you'll get a link-time error about multiple definitions. In C++, this not only works, it's preferred.
This is something that can easily trip up a C++ programmer who isn't used to C. C++ books tend to emphasize that #defines are bad and you should use consts instead.
I'm guessing that epa works with C++ more often than with C. :)
Posted Aug 5, 2016 23:47 UTC (Fri)
by Jonno (subscriber, #49613)
[Link] (2 responses)
Posted Aug 6, 2016 19:16 UTC (Sat)
by xtifr (guest, #143)
[Link]
The difference may be less than you think, though. You can make a pointer to a const int (and this should prevent the compiler from optimizing away the storage), but the same is true of an inline function. The real difference between an inline and a static is that a static will have a separate copy for each translation unit, while an inline will only have one copy for the whole program. (Assuming they aren't all optimized away.)
For normal use (no pointers), the difference won't matter, since the storage will be elided if you turn on even the most basic optimizations (which is pretty normal). And, since you can't modify a const int even with a cast (unless I've just found a bug in g++), the only practical difference would be if you tried to compare two different pointers to what you thought was the same const int. If the const is static, then the comparison *might* fail; if it's inline, it would succeed. But that's not something I've ever done or seen done, so I honestly don't know what the right answer is.
Anyway, I suspect we're drifting a bit off topic, and what you say sounds plausible enough, so I'm certainly not going to contradict you.
Posted Aug 13, 2016 2:38 UTC (Sat)
by thestinger (guest, #91827)
[Link]
Posted Aug 5, 2016 7:11 UTC (Fri)
by k8to (guest, #15413)
[Link] (1 responses)
Theoretically, in another language, the promise that it is const could be strong enough that whole-program optimization could eventually fold away its existence as a memory-located value, but C doesn't give promises that strong for const items.
Placing the definition in a header doesn't help, then you get N definitions of the same-named item, all with global scope. I'm pretty sure this is an error, and in any event if it worked, you'd have the value in memory many times, once for each unit.
Declaring it extern in a header and providing it in one unit gets you one value in memory, but still you don't get the immediates. The meaning of such a symbol is a memory value.
If you declare it as static const int, then a relatively modern compiler will *usually* be able to fully optimize it. You can still break everything by taking the address of it anywhere, which will push it back into memory. Sometimes insufficiently experienced programmers will make this type of mistake, and de-optimize your carefully constructed const int, potentially causing problems later that can be difficult to spot.
The define is very simple to understand, by contrast, which is it's strength.
Posted Aug 5, 2016 13:32 UTC (Fri)
by epa (subscriber, #39769)
[Link]
The arguments for and against use of the preprocessor have often been covered. One common reason to prefer language constructs over macros is that they are easier for static analysis tools. I wonder whether the Sparse tool is able to track uses of a named constant defined with the preprocessor?
Posted Aug 5, 2016 7:27 UTC (Fri)
by mchapman (subscriber, #66589)
[Link] (2 responses)
I have frequently seen enum values used for constants instead of preprocessor macros or const variables. They end up in the program's debugging info, so you can use their names in gdb, but they don't require any memory in the process itself.
Posted Aug 5, 2016 13:55 UTC (Fri)
by andresfreund (subscriber, #69562)
[Link]
Posted Aug 8, 2016 1:39 UTC (Mon)
by JdGordy (subscriber, #70103)
[Link]
Posted Aug 5, 2016 13:30 UTC (Fri)
by Jonno (subscriber, #49613)
[Link]
Posted Aug 4, 2016 16:27 UTC (Thu)
by error27 (subscriber, #8346)
[Link]
The problem is that you have some false positives where people do "cmd = SET_TIMER | 10;" where 10 is the number of seconds. I have found a few bugs with this check but I haven't pushed it because of the false positives. Anyway, if anyone had introduced Al Viro's bug then I would probably have emailed them.
Posted Aug 16, 2016 14:53 UTC (Tue)
by marcH (subscriber, #57642)
[Link]
I've met many people who wrongly believe that octal can do everything and who don't realize that chmod can perform incremental changes like this - without any octal equivalent whatsoever:
chmod -R g=r foo/
File permissions in the kernel
File permissions in the kernel
Is there a reason these are done as File permissions in the kernel
#define instead of const int?
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
You're right, File permissions in the kernel
static const int is what's needed. But just taking the address of it, while it does require allocating four bytes of space for the integer, need not stop other uses of it from being an inlined constant. A quick experiment with gcc -O seems to confirm this.
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
File permissions in the kernel
But a static const int does not (assuming it's initializer is a constexpr, it's address is never taken, and optimizations are turned on or --fno-keep-static-consts is passed to gcc).
File permissions in the kernel
File permissions in the kernel
I think I've even seen at times attempts to re-implement commands like the above with a script...