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

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

[Joshua Abraham]

This patch fixes the checkpatch.pl warning:
"CHECK: multiple assignments should be avoided"

Signed-off-by: Joshua Abraham <j.abraham1776@gmail.com>
---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 0d8ed002adcb..384218946108 100644
--- 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;
 
 	for (i = 0; i < priv->num_fqs; i++) {
 		fq = &priv->fq[i];
-- 
2.15.0

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

[Greg KH]

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"
> 
> Signed-off-by: Joshua Abraham <j.abraham1776@gmail.com>
> ---
>  drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> index 0d8ed002adcb..384218946108 100644
> --- 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 :)

thanks,

greg k-h

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

[Dan Carpenter]

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"
> > 
> > Signed-off-by: Joshua Abraham <j.abraham1776@gmail.com>
> > ---
> >  drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > index 0d8ed002adcb..384218946108 100644
> > --- 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.

regards,
dan carpenter

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

[Joe Perches]

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"
> > > 
> > > Signed-off-by: Joshua Abraham <j.abraham1776@gmail.com>
> > > ---
> > >  drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > > index 0d8ed002adcb..384218946108 100644
> > > --- 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@oracle.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 switch/case indentation (with fixes)
    - add paragraph on multiple-assignments
    - add more on Braces
    - add section on Spaces; add typeof, alignof, & __attribute__ with sizeof;
      add more on postfix/prefix increment/decrement operators
    - add paragraph on function breaks in source files; add info on
      function prototype parameter names
    - add paragraph on EXPORT_SYMBOL placement
    - add section on /*-comment style, long-comment style, and data
      declarations and comments
    - correct some chapter number references that were missed when
      chapters were renumbered
    
    Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
    Acked-by: Jesper Juhl <jesper.juhl@gmail.com>
    Acked-by: Jan Engelhardt <jengelh@gmx.de>
    Signed-off-by: Andrew Morton <akpm@osdl.org>

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

[Randy Dunlap]

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"
>>>>
>>>> Signed-off-by: Joshua Abraham <j.abraham1776@gmail.com>
>>>> ---
>>>>  drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>>>> index 0d8ed002adcb..384218946108 100644
>>>> --- 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@oracle.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 switch/case indentation (with fixes)
>     - add paragraph on multiple-assignments
>     - add more on Braces
>     - add section on Spaces; add typeof, alignof, & __attribute__ with sizeof;
>       add more on postfix/prefix increment/decrement operators
>     - add paragraph on function breaks in source files; add info on
>       function prototype parameter names
>     - add paragraph on EXPORT_SYMBOL placement
>     - add section on /*-comment style, long-comment style, and data
>       declarations and comments
>     - correct some chapter number references that were missed when
>       chapters were renumbered
>     
>     Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>     Acked-by: Jesper Juhl <jesper.juhl@gmail.com>
>     Acked-by: Jan Engelhardt <jengelh@gmx.de>
>     Signed-off-by: Andrew Morton <akpm@osdl.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.


-- 
~Randy

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

[Joe Perches]

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@oracle.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@oracle.com>
> >     Acked-by: Jesper Juhl <jesper.juhl@gmail.com>
> >     Acked-by: Jan Engelhardt <jengelh@gmx.de>
> >     Signed-off-by: Andrew Morton <akpm@osdl.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;

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

[Josh Abraham]

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"
> > 
> > Signed-off-by: Joshua Abraham <j.abraham1776@gmail.com>
> > ---
> >  drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > index 0d8ed002adcb..384218946108 100644
> > --- 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 :)
> 
> thanks,
> 
> greg k-h

It does make a lot more sense.

I will trust checkpatch less, and my eyes more :)

-Josh