NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/50949: src/sys/dev/pcmcia/if_malo_pcmcia.c:637]: (style) Suspicious condition



> On Mar 11, 2016, at 12:20 PM, <dcb314%hotmail.com@localhost> <dcb314%hotmail.com@localhost> wrote:
> 
> src/sys/dev/pcmcia/if_malo_pcmcia.c:637]: (style) Suspicious condition (assignment + comparison); Clarify expression with parentheses.
> 
> 
> Source code is
> 
>   if ((error = ieee80211_media_change(ifp) != ENETRESET))
> 
> 
> Maybe better code
> 
>   if ((error = ieee80211_media_change(ifp)) != ENETRESET)

It's a matter of preference, I'll admit.  But I would much rather see such constructs written as two statements:

error = ieee80211_media_change(ifp);
if (error != ENETRESET) ....

Admittedly that doesn't work as well in "while" statements, for those the assign and test constructs are arguably helpful -- the alternative is a while (1) with a conditional break, which is definitely a bit more clutter.  But for "if" statements I'd rather see the two aspects broken out into two separate lines.

	paul


Home | Main Index | Thread Index | Old Index