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

[<Paul_Koning%Dell.com@localhost>]

The following reply was made to PR kern/50949; it has been noted by GNATS.

From: <Paul_Koning%Dell.com@localhost>
To: <gnats-bugs%NetBSD.org@localhost>, <dcb314%hotmail.com@localhost>
Cc: <kern-bug-people%netbsd.org@localhost>, <gnats-admin%netbsd.org@localhost>,
	<netbsd-bugs%netbsd.org@localhost>
Subject: Re: kern/50949: src/sys/dev/pcmcia/if_malo_pcmcia.c:637]: (style)
 Suspicious condition
Date: Fri, 11 Mar 2016 17:25:26 +0000

 > On Mar 11, 2016, at 12:20 PM, <dcb314%hotmail.com@localhost> <dcb314%hotmail.com@localhost> w=
 rote:
 >=20
 > src/sys/dev/pcmcia/if_malo_pcmcia.c:637]: (style) Suspicious condition (a=
 ssignment + comparison); Clarify expression with parentheses.
 >=20
 >=20
 > Source code is
 >=20
 >   if ((error =3D ieee80211_media_change(ifp) !=3D ENETRESET))
 >=20
 >=20
 > Maybe better code
 >=20
 >   if ((error =3D ieee80211_media_change(ifp)) !=3D ENETRESET)
 
 It's a matter of preference, I'll admit.  But I would much rather see such =
 constructs written as two statements:
 
 error =3D ieee80211_media_change(ifp);
 if (error !=3D ENETRESET) ....
 
 Admittedly that doesn't work as well in "while" statements, for those the a=
 ssign and test constructs are arguably helpful -- the alternative is a whil=
 e (1) with a conditional break, which is definitely a bit more clutter.  Bu=
 t for "if" statements I'd rather see the two aspects broken out into two se=
 parate lines.
 
 	paul