svn commit: r197654 - head/sys/dev/if_ndis
Bruce Evans
brde at optusnet.com.au
Thu Oct 1 18:20:31 UTC 2009
On Thu, 1 Oct 2009, Coleman Kane wrote:
> On Fri, 2009-10-02 at 00:36 +1000, Bruce Evans wrote:
>> On Thu, 1 Oct 2009, [utf-8] Dag-Erling Smørgrav wrote:
>>
>>> Coleman Kane <cokane at FreeBSD.org> writes:
>>>> - if (sc->ndis_80211 && vap)
>>>> + if ((sc->ndis_80211 != NULL) && (vap != NULL))
>>>
>>> sc->ndis_80211 is an int. NULL is a pointer.
>>
>> Also, the number of style bugs was doubled on (almost?) every changed line
>> by adding 2 sets of unnecessary parentheses.
>>
>> Bruce
>
> Re-read style(9) more closely.
Do I need to read it at all :-).
> Yes... the extra parentheses are superfluous, and should therefore be
> removed. However, the current rev, which looks like this:
>
> if ((sc->ndis_80211 != 0) && (vap != NULL))
>
> doesn't help the author shoot themselves in the foot as violating the
> "explicitly compare values to zero" rule did in the earlier revision.
Actually I needed to count the style bugs more carefully -- 2 implicit
comparisons with 0 or NULL (unless the first one is really boolean),
but I only counted 1, while I counted 2 for the extra parentheses.
> I'll heed the request of the second-to-last paragraph of style(9) on
> this particular change, not churning the SVN repo further, and make a
> mental note for later.
Thanks. I forgot about that paragraph being there. I think churning
repos doesn't matter much now if it ever did, but churning sources makes
their history hard to understand.
Bruce
More information about the svn-src-all
mailing list