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

From: Joe Perches
Date: Wed Nov 08 2017 - 12:30:50 EST


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@xxxxxxxxxx>
> > 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@xxxxxxxxxx>
> > Acked-by: Jesper Juhl <jesper.juhl@xxxxxxxxx>
> > Acked-by: Jan Engelhardt <jengelh@xxxxxx>
> > Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
>
> 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;