[PATCH] staging: rtl8192u: bool tests don't need comparisons

Frans Klaver fransklaver at gmail.com
Tue Jun 23 13:37:20 UTC 2015


On Tue, Jun 23, 2015 at 3:21 PM, Luis de Bethencourt
<luis at debethencourt.com> wrote:

>> >         if (dm_digtable.dig_algorithm_switch) {
>> > @@ -3062,7 +3062,8 @@ static void dm_dynamic_txpower(struct net_device *dev)
>> >                         priv->bDynamicTxLowPower = false;
>> >                 } else {
>> >                         /* high power state check */
>> > -                       if (priv->undecorated_smoothed_pwdb < txlowpower_threshold && priv->bDynamicTxHighPower == true)
>> > +                       if (priv->undecorated_smoothed_pwdb <
>> > +                           txlowpower_threshold && priv->bDynamicTxHighPower)
>> >                                 priv->bDynamicTxHighPower = false;
>>
>> Oh, this has a misleading air hanging over it. It focuses the eyes on
>> "txlowpower_threshold && priv->bDynamicTxHighPower", while that
>> probably isn't the intent.
>>
>> Frans
>
> I agree, and wasn't sure what the best way to deal with was.
>
> The following doesn't mislead but goes above 80 characters.
>                         if (priv->undecorated_smoothed_pwdb < txlowpower_threshold &&
>                             priv->bDynamicTxHighPower == true)
>
> It is better than the original but it doesn't completely fix it.
>
> If this is a better compromise I can update the patch.

If we keep people's internal parsers working properly, I think having
a line of three characters too long is a fair compromise. Besides
that, there are a lot more lines of code in that file that need to be
brought back to under 80 characters.

If you really care about that line length, precede with a patch (or
two) that changes those insanely long (local!) variable names, so that
you can break up the line right away.

Have fun,
Frans


More information about the devel mailing list