
Re: [PATCH 1115/1285] Replace numeric parameter like 0444 with macro - sslalready
https://lkml.org/lkml/2016/8/2/1728
======
DblPlusUngood
Linus' reply: "I detest these patches."

[https://lkml.org/lkml/2016/8/2/1589](https://lkml.org/lkml/2016/8/2/1589)

~~~
jlgaddis
> _" I don't want to see _any_ of these patches being applied."_

~~~
totony
>"And part of it is exactly that there are nineteen billion of these "random
letter combinations" "

------
abstractbeliefs
Another option to understand the magnitude of this fuck up is to have a look
at today's chronological email list:
[https://lkml.org/lkml/2016/8/2/](https://lkml.org/lkml/2016/8/2/)

~~~
bjackman
My favourite thing about it is that there was no header email (usually [PATCH
0/n] is just text with an overview of the following patches. Then any top-
level discussion (I.e. "NAK: octal is more readable") takes place in replies
to that email.

Instead here there are 1285 separate threads of discussion with dozens of
people all saying the same thing with no easy way to discover it's already
been said (and at least one maintainer has replied to every single one he's
CC'd on). Beautiful carnage.

------
SloopJon
It's an interesting question as to which is more readable:

    
    
        -module_param(max_sets, int, 0600);
        +module_param(max_sets, int, S_IRUSR | S_IWUSR);
    

However, I'm missing context. Did someone at Intel really spam 1,285 patches
without any prior discussion?

~~~
MichaelGG
Seems like it. I would hope Intel wouldn't hire someone that would think this
is OK ... OTOH see Uber's writeup about how developers can't be expected to
think about transactions.

Maybe there's some perverse incentive at Intel? Like performance review that
considers how many patches you've sent? Or maybe its a broken tool to help
submit patches?

~~~
emeraldd
I'd be inclined to think it was a commit per smaller change set and each
commit got turned into a separate patch.

~~~
eridius
It looks like it got turned into one commit per file. Which is pretty
atrocious.

~~~
tedunangst
Isn't "one commit per independent change" common advice in git tutorials? Each
file can be changed independently of any other, so taken literally, exactly
how you're supposed to do it.

~~~
eridius
That's wildly incorrect. "One commit per logical change" would mean that every
single file modified in this way should have been in a single commit. By your
logic, you could _never_ have two files changes in the same commit.

~~~
mappu
_> you could never have two files changes in the same commit._

We're going OT here, but i don't think that applies to C, where a "logical
change" to a function definition may be split over two files (.c/.h).

------
jxy
I can understand that for someone has no background or long term use of
chmod(1) would need some effort to decipher what 0644 actually means. However,
for people who are well versed in this aspect, the numerical code is much
easier to parse.

This reminds me of constantly looking up j dict [0] before I could remember
what each number means. J's foreign conjunction is certainly an extreme here,
but it teaches me that what is easy to parse really depends on one's
proficiency in the language.

[0]
[http://www.jsoftware.com/help/dictionary/dx001.htm](http://www.jsoftware.com/help/dictionary/dx001.htm)

~~~
david-given
I've been using Unix for years _and years_ on a variety of systems, including
various sysadminy roles, and I _still_ cannot keep the octal permission
numbers in my head. My current job involves using one tool which only supports
the octal and I always go to an online site which calculates them for me
because otherwise I will just get them wrong.

I'd vastly prefer the symbolic forms (and am slightly surprised that the
kernel source has raw octal in it at all). Although I wouldn't have tried to
implement the change as a thousand separate patches...

~~~
ajdlinux
I'm the exact opposite - I can instantly understand what 600, 777, 664 and so
on mean with barely a thought, but having used Linux for the majority of my
life I still struggle to remember "chmod u+w" and whatever.

~~~
sanderjd
Those are different things though. "u+w" doesn't require me to know what the
current permissions are, but only that I want to let the owner write.

------
pavlov
The octal representation is such a deep-grained part of Unix on all levels
(both C and shell) that replacing it is futile. These weird 0-prefixed octal
constants will still be in common use a hundred years from now, for better and
worse.

Also, it's not easy to argue that "S_IRUSR | S_IRGRP | S_IROTH" is any kind of
readability improvement...

------
cm3
Linus is right because these are actually well known and easy to parse.

However, if instead of S_IRUSR|S_IWUSR (which I'd have to lookup up to parse
correctly), it were possible to write "a-rwx"|"u+rw", it would be even easier
to parse, without the need to memorize common octal numbers. In C, this could
be achieved with the preprocessor, I believe, if you create a sufficiently
smart CPP macro, especially since Linux is no stranger to heavy preprocessor
use.

~~~
jxy
Yes, you can certainly achieve that with setmode(3) and getmode(3).

~~~
ecma
Not in the kernel, you can't :)

------
Artlav
Sounds like someone ran a script without thinking too much.

------
0x0
Shameful to see @intel.com in the signed-off-by on this spam train

~~~
cm3
Why is it shameful? Someone considered this an improvement, went through the
process of creating >1000 patches and submitted them for review. That the
patches were rejected categorically and someone presumably spent consideralbe
development time on them is also not bad, because the likelihood of a good
response to a question "what if I replaced .. with .." is low and seeing the
huge patchset also drives home the amount of magic numbers one might replace.

It's just how software is developed where code is reviewed before inclusion
and not committed-by-default and backed out later. And there's no Intel pre-
mailing patch review group that has to vet kernel patches before a dev is
allowed to send them out, ignoring NDA'ed things, of course.

There is no ill will here, and Intel has more unseasoned Linux developers now,
so it's normal for seemingly naive patches to appear.

We need more developers who try, fail, try again, and that requires less
shaming and more encouragement.

~~~
0x0
Blasting over 1200 emails to a mailing list with no heads-up appears quite
tone-deaf, to put it mildly even if you presume good intentions.

Edit: The impact could be shown in a single email with a diffstat, for
example. Complaining about a lack of feedback for such a thing sounds more
like complaining about failing to draw others in to a bikeshedding
competition.

~~~
digi_owl
Or a initial email going "heads up, big patch set coming through!", with the
patch set as replies to that one.

~~~
cm3
That's usually what happens with a git formatted patchset that has you draft a
patch-0 (aka introduction of the series) email.

------
raldi
The macros are for when you're _reading_ the value, not setting it:

    
    
        if(m & S_IRUSR) { ... }

~~~
tedunangst
That doesn't make a lot of sense. If you know the octal numbers, you don't
forget them when reading the value.

------
chj
These numbers are almost hardwired in my brain and I am not even a kernel
developer. I think most kernel developers should not need the help of these
magic macros.

------
jlgaddis
I'd be curious to see a graph of the load average on vger.kernel.org today. I
can imagine a huge spike lasting several hours.

------
YeGoblynQueenne
Why did each of those need to be a simple patch? Couldn't they be just one big
patch, which would have been a little easier to read?

(kernel muggle here, mind)

~~~
ajdlinux
They didn't. Many maintainers will NAK purely because of that.

