Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 7 May 2021 08:24:33 +0300
From:   Alexey Dobriyan <adobriyan@gmail.com>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [patch 04/91] proc: save LOC in __xlate_proc_name()
Message-ID: <YJTPEXsvTs7QXIBx@localhost.localdomain>
References: <20210506180126.03e1baee7ca52bedb6cc6003@linux-foundation.org>
 <20210507010213.V8MhqooKS%akpm@linux-foundation.org>
 <CAHk-=wjKQZczi-J0rEUoPS+=Q6gCSpr3UmWzh-L8Qs9WGGUHuA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAHk-=wjKQZczi-J0rEUoPS+=Q6gCSpr3UmWzh-L8Qs9WGGUHuA@mail.gmail.com>
Precedence: bulk

On Thu, May 06, 2021 at 07:24:36PM -0700, Linus Torvalds wrote:
> On Thu, May 6, 2021 at 6:02 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > From: Alexey Dobriyan <adobriyan@gmail.com>
> > Subject: proc: save LOC in __xlate_proc_name()
> ..
> > +       while ((next = strchr(cp, '/'))) {
> 
> Please don't do this.

It is actually how it should be done.

Kernel has such code in other places

	#define hlist_for_each(pos, head) \
	        for (pos = (head)->first; pos ; pos = pos->next)

And we do check pointers for validness like this

	if (ptr) {
	}

"while" loop is no different.

> Yes, gcc suggests that double parentheses syntax around an assignment
> to avoid warnings.

I never saw this warning. I just wrote double parenth knowing the
warning will be emitted. It's an old warning.

> gcc is wrong, and is being completely stupid.

> The proper way to avoid the "assignment in conditional" warning is to
> (surprise, surprise) USE A CONDITIONAL.
> 
> So that
> 
>           while ((next = strchr(cp, '/'))) {
> 
> is the crazy rantings of a misguided compiler. No sane human should
> ever care about some odd double parenthesis syntax. We're not writing
> LISP, for chrissake.
> 
> The proper way to write this is
> 
>           while ((next = strchr(cp, '/')) != NULL) {

This NULL is redundant in the same way "if (ptr != NULL)" is redundant.
Even more so in C where comparison can't be overloaded.
You don't even save the parenth.

> which makes sense to not just a machine, but to a human, and avoids
> the whole "assignment used as a conditional" warning very naturally.
> 
> See? Now it uses a conditional as a conditional. Doesn't that make a
> whole lot more sense than the crazy ramblings of a broken machine
> mind?
> 
> I fixed it up manually, I just wanted to rant against this kind of
> "mindlessly take advice from the compiler without thinking about it".

Whatever-by: Alexey Dobriyan <adobriyan@gmail.com>