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

[Douglas Gilbert <dgilbert@interlog.com>]

On 2016-04-26 02:13 PM, 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.

Hmmm. I hadn't even noticed the bitwise ORs. Not sure why
Martin did that. Anyway I can change it.

BTW I had no intention of changing Martin's LB provisioning logic.
Those lines appear in the patch due to the name shortening.

>> +    sdebug_verbose = !!(SDEBUG_OPT_NOISE & sdebug_opts);
>> +    sdebug_any_injecting_opt = !!(SDEBUG_OPT_ALL_INJECTING & sdebug_opts);
>
> Same comment here: please put constants at the right side. I think the latest
> version of checkpatch requests to do so.

The whole patch series went through checkpatch.pl without
errors or warnings on linux-stable.git .

Doug Gilbert