From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44517) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XD3C5-0003Ww-Ko for qemu-devel@nongnu.org; Thu, 31 Jul 2014 23:07:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XD3C1-0004VC-7X for qemu-devel@nongnu.org; Thu, 31 Jul 2014 23:07:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33004) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XD3C0-0004Um-WB for qemu-devel@nongnu.org; Thu, 31 Jul 2014 23:07:41 -0400 Message-ID: <53DB0458.5020605@redhat.com> Date: Thu, 31 Jul 2014 21:07:04 -0600 From: Eric Blake MIME-Version: 1.0 References: <1406860365-5516-1-git-send-email-arei.gonglei@huawei.com> <1406860365-5516-2-git-send-email-arei.gonglei@huawei.com> In-Reply-To: <1406860365-5516-2-git-send-email-arei.gonglei@huawei.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="RMgI77lEfeIt1L90CjlfW9olDjN5EuKHP" Subject: Re: [Qemu-devel] [PATCH v2 1/8] CODING_STYLE: Section about conditional statement List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: arei.gonglei@huawei.com, qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, weidong.huang@huawei.com, stefanha@redhat.com, mst@redhat.com, marcel.a@redhat.com, luonengjun@huawei.com, armbru@redhat.com, lcapitulino@redhat.com, av1474@comtv.ru, kraxel@redhat.com, aliguori@amazon.com, imammedo@redhat.com, dmitry@daynix.com, pbonzini@redhat.com, peter.huangpeng@huawei.com, afaerber@suse.de, dgilbert@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --RMgI77lEfeIt1L90CjlfW9olDjN5EuKHP Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/31/2014 08:32 PM, arei.gonglei@huawei.com wrote: > From: Gonglei >=20 > Yoda conidtions lack of readability, and QEMU have a s/conidtions/conditions/ s/of // s/have/has/ > strict compiler configuration for checking a common > mistake like "if (dev =3D NULL)". Make it a written rule. >=20 > Signed-off-by: Gonglei > --- > CODING_STYLE | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) >=20 > diff --git a/CODING_STYLE b/CODING_STYLE > index 4280945..11a79d4 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -91,3 +91,22 @@ Mixed declarations (interleaving statements and decl= arations within blocks) > are not allowed; declarations should be at the beginning of blocks. I= n other > words, the code should not generate warnings if using GCC's > -Wdeclaration-after-statement option. > + > +6. Conditional statement s/statement/statements/ > + > +Please don't use Yoda conditions because of lack of readability. Furth= ermore, > +it is not the QEMU idiomatic coding style. Example: > + > +Usually a conditional statement in QEMU would be written as: > +if (a =3D=3D 0) { > + /* Reads like: "If a is equal to 0..." */ > + do_something(); > +} > + > +Yoda conditions describe the same expression, but reversed: > +if (0 =3D=3D a) { > + /* Reads like: "If 0 equals to a" */ > + do_something(); > +} > + > +The constant is listed first, then the variable being compared to. >=20 This spends more lines documenting the bad style than the good, and doesn't quite flow with the rest of the document. At the risk of sounding like a complete rewrite, how about: =3D=3D=3D=3D=3D When comparing a variable for (in)equality with a constant, list the constant on the right, as in: if (a =3D=3D 0) { do_something(); } Rationale: Yoda conditionals (as in 'if (0 =3D=3D a)') are awkward to rea= d. Besides, good compilers already warn users when =3D=3D is mis-typed as =3D= , even when the constant is on the right. =3D=3D=3D=3D=3D and maybe some other ideas also worth adding: =3D=3D=3D=3D=3D Avoid redundant comparisons: (bool_expr =3D=3D true) is better written as= (bool_expr), and (ptr =3D=3D NULL) is shorter as (!ptr). Use of !!value = is a convenient shorthand for converting a value into a boolean. =3D=3D=3D=3D=3D --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --RMgI77lEfeIt1L90CjlfW9olDjN5EuKHP 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 iQEcBAEBCAAGBQJT2wRYAAoJEKeha0olJ0NqVvUH/i2gCfvWlDtlcp0sVNOAaDkI qoZXQktUBFIJndkpAlXAp6YRGutodEmE8OE9ZL+BeMC7V2LpoVS1BIHo9eIjdR3w kGS8aKF3cFl9rmGEZvWU0MvKhyUHhi3QDeNjoY2nTwukulA46/wSuniG/9KHVWEz /mZ6jGA0ZC+An0VlAE96lArJqnmagBVTKpvF14yZLZEP8kFmCywLXykE9N2j101Y QwI2MsKt66QGitQX0FFILcbstiXvDBVbyPxorhqXgPPuhO/t9qDn2mJACX1BQkJt JSIlG1cyuCdhwNcGiwX7HSRxq+mgZjBu5hwKqeSLEquBpuPl7KIB9KzEbQ+AJd4= =W1GF -----END PGP SIGNATURE----- --RMgI77lEfeIt1L90CjlfW9olDjN5EuKHP--