linux-kernel - Re: [patch 04/91] proc: save LOC in __xlate_proc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 6 May 2021 23:34:28 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Alexey Dobriyan <adobriyan@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [patch 04/91] proc: save LOC in __xlate_proc_name()

On Thu, May 6, 2021 at 10:24 PM Alexey Dobriyan <adobriyan@...il.com> 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