linux-kernel - Re: [PATCH] staging: fsl-dpaa2: Fix multiple assignments should be avoided

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]

Message-ID: <1510162241.15768.26.camel@perches.com>
Date:   Wed, 08 Nov 2017 09:30:41 -0800
From:   Joe Perches <joe@...ches.com>
To:     Randy Dunlap <rdunlap@...radead.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Andy Whitcroft <apw@...dowen.org>
Cc:     Joshua Abraham <j.abraham1776@...il.com>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] staging: fsl-dpaa2: Fix multiple assignments should be
 avoided

On Wed, 2017-11-08 at 09:16 -0800, Randy Dunlap wrote:
> On 11/08/2017 03:39 AM, Joe Perches wrote:
> > On Wed, 2017-11-08 at 12:40 +0300, Dan Carpenter wrote:
> > > On Wed, Nov 08, 2017 at 10:20:48AM +0100, Greg KH wrote:
> > > > On Tue, Nov 07, 2017 at 07:45:03PM -0500, Joshua Abraham wrote:
> > > > > This patch fixes the checkpatch.pl warning:
> > > > > "CHECK: multiple assignments should be avoided"
[]
> > > > > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
[]
> > > > > @@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv *priv)
> > > > >  	 * This may well change at runtime, either through irqbalance or
> > > > >  	 * through direct user intervention.
> > > > >  	 */
> > > > > -	rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask);
> > > > > +	rx_cpu = cpumask_first(&priv->dpio_cpumask);
> > > > > +	txc_cpu = rx_cpu;
> > > > 
> > > > The original code here makes much more sense, doesn't it?
> > > > 
> > > > Sometimes checkpatch is wrong :)
> > > 
> > > It feels like the majority of these multiple assignment warnings are
> > > wrong.  I thought it would be a good idea at first but after looking at
> > > a bunch of the patches it feels like we should just remove the check.
> > 
> > I don't have a particular opinion one way or another.
> > 
> > That bit was added to CodingStyle by Randy Dunlap back
> > in 2006 by
> > 
> > commit b3fc9941fbc6efe5cb77728adb0fb12be363e73e
> > Author: Randy Dunlap <randy.dunlap@...cle.com>
> > Date:   Sun Dec 10 02:18:56 2006 -0800
> > 
> >     [PATCH] CodingStyle updates
> >     
> >     Add some kernel coding style comments, mostly pulled from emails
> >     by Andrew Morton, Jesper Juhl, and Randy Dunlap.
[]
> >     - add paragraph on multiple-assignments
[]
> >     Signed-off-by: Randy Dunlap <randy.dunlap@...cle.com>
> >     Acked-by: Jesper Juhl <jesper.juhl@...il.com>
> >     Acked-by: Jan Engelhardt <jengelh@....de>
> >     Signed-off-by: Andrew Morton <akpm@...l.org>
> 
> So it was.  I would certainly support removing it.
> 
> I suspect I copied that from someone else's email.  My personal style
> would be to allow that.

My personal style too actually.

	foo = bar = baz();

is rather more sensible to me than

	bar = baz();
	foo = bar;

I think this style stems from the desire to be similar to
single line definitions, but if the identifier lengths
are short it really doesn't make the code better to read.

Though I personally do prefer
	int i;
	int *j;
over
	int i, *j;