Re: [patch 04/91] proc: save LOC in __xlate_proc_name()
From: Linus Torvalds
Date: Fri May 07 2021 - 02:34:51 EST
On Thu, May 6, 2021 at 10:24 PM Alexey Dobriyan <adobriyan@xxxxxxxxx> wrote:
>
> On Thu, May 06, 2021 at 07:24:36PM -0700, Linus Torvalds wrote:
> > ..
> > > + while ((next = strchr(cp, '/'))) {
> >
> > Please don't do this.
>
> It is actually how it should be done.
No, Alexey, it really isn't.
> Kernel has such code in other places
.. and then you show that you do not understand the problem at all.
It's the unacceptable "double parentheses" disease I'm talking about.
The "other places" you bring up DO NOT DO THAT.
> "while" loop is no different.
This is not at all about the while loop. Comprende?
We don't do
if ((a = b)) ..
or
while ((a = b)) ..
or anything like that.
Why? Because that is pure and utter crazy garbage. The above syntax is
just insane.
> I never saw this warning. I just wrote double parenth knowing the
> warning will be emitted. It's an old warning.
Yes, and you picked the wrong solution for it. You picked the "write
bad code just to make the warning go away".
We don't make warnings go away by writing insane and bad code. We
write sane code.
So I repeat:
> > The proper way to write this is
> >
> > while ((next = strchr(cp, '/')) != NULL) {
Notice the difference?
Because the "hide a warning by adding random parentheses" is not the
way we do things in the kernel.
So yes, we also write
while (next) {..
and you are correct that this case doesn't need the " != NULL".
But notice how it also doesn't need the crazy extra parentheses?
Linus