Re: [PATCH 01/12] scsi_debug: cleanup naming and bit crunching

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Tue, 2016-04-26 at 11:13 -0700, Bart Van Assche wrote:
> On 04/25/2016 09:16 AM, Douglas Gilbert wrote:
> > @@ -580,8 +580,8 @@ static int sdebug_sectors_per;		/
> > * sectors per cylinder */
> > 
> >   static unsigned int scsi_debug_lbp(void)
> >   {
> > -	return ((0 == scsi_debug_fake_rw) &&
> > -		(scsi_debug_lbpu | scsi_debug_lbpws |
> > scsi_debug_lbpws10));
> > +	return ((0 == sdebug_fake_rw) &&
> > +		(sdebug_lbpu | sdebug_lbpws | sdebug_lbpws10));
> >   }
> 
> Since you are changing this code, please remove the superfluous 
> parentheses, place the constant at the right side of the comparison 
> and change "|" into "||" since the intention of the above code is to
> perform a logical or and this code is not in a performance-critical
> path.

This is getting completely pointless.  We're not slaves to checkpatch
and the Maintainer has considerable leeway to decide the style for
their code.  I personally don't like the const == rvalue form of compar
ison, but it does pick up an erroneous '=' instead of '==', so I'm not
going to object to a maintainer who wishes to use it.

Plus, if you only alter it in one place, the file becomes a mess of
styles and thus harder to read.

James