From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38511) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkEW7-0003iG-Mr for qemu-devel@nongnu.org; Tue, 13 May 2014 11:21:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkEW2-0002Eq-MV for qemu-devel@nongnu.org; Tue, 13 May 2014 11:21:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31966) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkEW2-0002Ef-FZ for qemu-devel@nongnu.org; Tue, 13 May 2014 11:21:14 -0400 Message-ID: <53723867.1020409@redhat.com> Date: Tue, 13 May 2014 09:21:11 -0600 From: Eric Blake MIME-Version: 1.0 References: <1399964572-5376-1-git-send-email-marc.mari.barcelo@gmail.com> <1399964572-5376-17-git-send-email-marc.mari.barcelo@gmail.com> In-Reply-To: <1399964572-5376-17-git-send-email-marc.mari.barcelo@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="BtrAgVoNPwThWxKvAKpAaNASvLC6MBL5x" Subject: Re: [Qemu-devel] [PATCH v2 16/16] common: Convert conditional compilation of debug printfs to regular ifs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?TWFyYyBNYXLDrQ==?= , qemu-devel@nongnu.org Cc: Stefan Hajnoczi , Peter Crosthwaite , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --BtrAgVoNPwThWxKvAKpAaNASvLC6MBL5x Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 05/13/2014 01:02 AM, Marc Mar=C3=AD wrote: > Modify debug macros to have the same format through the codebase and us= e regular > ifs instead of ifdef. >=20 > As the debug printf is always put in code, some casting had to be added= to avoid > warnings treated as errors at compile time. >=20 > Signed-off-by: Marc Mar=C3=AD > --- > include/qemu-common.h | 7 +++++++ > migration-rdma.c | 32 ++++++++++++++------------------ > page_cache.c | 10 ++++++---- > 3 files changed, 27 insertions(+), 22 deletions(-) >=20 > diff --git a/include/qemu-common.h b/include/qemu-common.h > index 3f3fd60..3593bdc 100644 > --- a/include/qemu-common.h > +++ b/include/qemu-common.h > @@ -463,3 +463,10 @@ int parse_debug_env(const char *name, int max, int= initial); > const char *qemu_ether_ntoa(const MACAddr *mac); > =20 > #endif > + > +#define QEMU_DPRINTF(cond,pfx,fmt,...) \ > + do { \ > + if (cond) { \ > + fprintf(stderr, pfx": %s:"fmt, __func__, ## __VA_ARGS__); \ Another instance of using the gcc extension ##__VA_ARGS__, when you could be portable to all C99 compilers by just subsuming fmt into the '..= =2E'. Spacing doesn't match normal style; need space after comma, and use indentation of 4 spaces not 2. It should be: #define QEMU_DPRINTF(cond, pfx, fmt, ...) \ do { \ if (cond) { \ =2E..... I'm not sure whether the code base has a definite preference towards lining up \ continuations in the same column. I agree with the other review comments that this hunk needs to be first in the series - each patch must pass 'make' on its own, but your series as sent fails to compile earlier patches until this hunk is in. > */ > #define ERROR(errp, fmt, ...) \ > - do { \ > - fprintf(stderr, "RDMA ERROR: " fmt "\n", ## __VA_ARGS__); \ > - if (errp && (*(errp) =3D=3D NULL)) { \ > - error_setg(errp, "RDMA ERROR: " fmt, ## __VA_ARGS__); \ > - } \ > - } while (0) > + QEMU_DPRINTF(1, "RDMA ERROR", fmt"\n", ## __VA_ARGS__); \ > + do { if (errp && (*(errp) =3D=3D NULL)) { error_setg(errp, "RDMA E= RROR: " fmt, ## __VA_ARGS__); } } while (0) Once again, you broke a one-statement macro into multiple statements. Fix the placement of "do {". Pre-existing, but you might as well simplify the code to drop redundant () and comparison to NULL: if (errp && !*errp) { --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --BtrAgVoNPwThWxKvAKpAaNASvLC6MBL5x 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 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTcjhoAAoJEKeha0olJ0NqHbgIAJimHbURjAChwA3vtx1ntSYR HDmx+AW4jw6sNPWzuGMr2Rm012Z9kYdzlnLiIgDqWkiRcf6ye+STa+6y9YQ0uXT7 iUOqFKg2MgPk3FkV8KaqErLIDTdNJOuhosCqhoTYwEX4tU9zCl8XAmyzzM+8Dei4 0XTnH4B8rOOJ2+2rDDhun43kthyyYF9vc6PNHYG+hu16yh4B6aPt/PG0LqZlH4J9 pSDjtis78+tBZ+GkhOn1rBiY5x+qHcDIbfC43UQB8Qdncw+VNFdZ4Q8j12aqFekz /vy009yTEJUjJUVzouhzMLQgNgZhauj5qhxfCjpCJ2SFYjztDnD17OodDmxkIR4= =ns+P -----END PGP SIGNATURE----- --BtrAgVoNPwThWxKvAKpAaNASvLC6MBL5x--