From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51648) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XDFD5-0002sS-FU for qemu-devel@nongnu.org; Fri, 01 Aug 2014 11:57:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XDFD0-000802-Gm for qemu-devel@nongnu.org; Fri, 01 Aug 2014 11:57:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17770) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XDFD0-0007z5-9Q for qemu-devel@nongnu.org; Fri, 01 Aug 2014 11:57:30 -0400 Message-ID: <53DBB8B4.6040604@redhat.com> Date: Fri, 01 Aug 2014 09:56:36 -0600 From: Eric Blake MIME-Version: 1.0 References: <1406860365-5516-1-git-send-email-arei.gonglei@huawei.com> <1406860365-5516-3-git-send-email-arei.gonglei@huawei.com> <53DB0499.2010505@redhat.com> <33183CC9F5247A488A2544077AF1902086C1E41E@SZXEMA503-MBS.china.huawei.com> <53DB0CB1.9040207@suse.de> <87fvhglnjl.fsf@blackfin.pond.sub.org> In-Reply-To: <87fvhglnjl.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="QBaMX9HDHjlf5AgHFdrRWqEbcsbPTX17P" Subject: Re: [Qemu-devel] [PATCH v2 2/8] usb: a trivial code change for more idiomatic writing style List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: "peter.maydell@linaro.org" , "peter.crosthwaite@xilinx.com" , "Huangweidong (C)" , "aliguori@amazon.com" , "mst@redhat.com" , "marcel.a@redhat.com" , Luonengjun , "qemu-devel@nongnu.org" , "lcapitulino@redhat.com" , "Gonglei (Arei)" , "av1474@comtv.ru" , "kraxel@redhat.com" , "stefanha@redhat.com" , "pbonzini@redhat.com" , "dmitry@daynix.com" , "imammedo@redhat.com" , "Huangpeng (Peter)" , "dgilbert@redhat.com" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --QBaMX9HDHjlf5AgHFdrRWqEbcsbPTX17P Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 08/01/2014 12:41 AM, Markus Armbruster wrote: >>>>> + if (data =3D=3D NULL) { >>>> >>>> Wouldn't it be even more idiomatic as: >>>> >>>> if (!data) { >>>> >>>> Probably applies throughout your series. >>>> >>> OK, will do. Thanks! >> >> Not so quick! You are free to use that in your patches, but please don= 't >> change all code that way without the author's consent. Just like "equa= ls >> null" is a natural English way of reading, compared to "null equals >> something", "not null" reads like a boolean expression to me, and even= >> worse while all valid C, "not strcmp" leads to mind-boggling inverted >> logic... If it's going to be controversial, then the right thing to do is that both '(!ptr)' and '(ptr =3D=3D NULL)' are acceptable, and that preference= should be given to consistency to nearby code. >=20 > !strcmp() is somewhat error prone, because it suggests inequality. That's why libvirt uses a STREQ() macro; it evaluates to !strcmp under the hood, but it's a LOT easier to reason about 'STREQ(a, b)' than it is to reason about '!strcmp(a, b)'. > Can't claim that for !data. That one suggests "no data", which is > exactly right. Like Eric, I prefer it to the cumbersome data =3D=3D NU= LL. I also prefer 'if (ptr)' over 'if (ptr !=3D NULL)'. > data =3D=3D 0 is right out. Correct, especially if data is a pointer (hmm, our coding style should mention that we STRONGLY prefer NULL over 0 when referring to the null pointer). >=20 > Since there's no consensus on !data vs. data =3D=3D NULL, you're free t= o > follow your own taste in new code. When changing existing code, imitat= e > nearby code. When nearby code is inconsistent, it's your own taste > again. Yes, so capturing this sentiment in the coding style guide would be worthwhile. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --QBaMX9HDHjlf5AgHFdrRWqEbcsbPTX17P Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJT27i0AAoJEKeha0olJ0NqyFgH/iwsghThy3HqjV+G52OaUovd AVRRlPFXhUg2nVJHetcVFX3Uzte5FCFp0mXfVMjzmMwqNrv6CDM5U0ErJgftHw0q c2KWFIT8Akp4YbIRpTmyim47bVtwYQXXwQ4fFWpMTsS30HEPMvGpkLdAm7Ohsm/3 hm01FT900lUqX8SA0uXPnq77D3hsyw7D2XPi8vWMzhg6XzuQm74U74mG60+vATt4 uFFt84XK7ePLLeTV6a5/nTMuq55X5+aG5I9a+FW9nyTPXS/LrIOGAD/+rO/PGvWF /Sx9H2qUuoYSTqtl1ajhrrYnMSzpPX2QLO5kOG1BanxUaKV9f+u0zLbHP6ppDxc= =kR3L -----END PGP SIGNATURE----- --QBaMX9HDHjlf5AgHFdrRWqEbcsbPTX17P--