Re: [patch 04/91] proc: save LOC in __xlate_proc_name()

[Alexey Dobriyan]

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@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > From: Alexey Dobriyan <adobriyan@xxxxxxxxx>
> > 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@xxxxxxxxx>