From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36391) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkER5-0000Uq-M5 for qemu-devel@nongnu.org; Tue, 13 May 2014 11:16:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkEQx-0000K8-2H for qemu-devel@nongnu.org; Tue, 13 May 2014 11:16:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:16214) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkEQw-0000Iy-Gl for qemu-devel@nongnu.org; Tue, 13 May 2014 11:15:58 -0400 Message-ID: <5372372A.7010709@redhat.com> Date: Tue, 13 May 2014 09:15:54 -0600 From: Eric Blake MIME-Version: 1.0 References: <1399964572-5376-1-git-send-email-marc.mari.barcelo@gmail.com> <1399964572-5376-2-git-send-email-marc.mari.barcelo@gmail.com> In-Reply-To: <1399964572-5376-2-git-send-email-marc.mari.barcelo@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="GqBIbKauJXsCjP5OlLTq4LKGIiCs14W5W" Subject: Re: [Qemu-devel] [PATCH v2 01/16] x86: 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 , xen-devel@lists.xensource.com, =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , kvm@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --GqBIbKauJXsCjP5OlLTq4LKGIiCs14W5W 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. Umm, where in this patch did you add casting? Don't add bogus lines to the commit message (I was assuming that it might be a case where the code has type mismatches, and where something like PRId32 would be better than adding a cast - but couldn't find where that was. Maybe other patches in the series are affected?) >=20 > Signed-off-by: Marc Mar=C3=AD > --- > -#define DEBUG(fmt, ...) \ > - do { \ > - fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__); \ > - } while (0) The old code was line-wrapped to fit in 80 columns... > +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 1 > #else > -#define DEBUG(fmt, ...) > +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 0 > #endif > =20 > +#define DEBUG(fmt, ...) QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED, = "pci_assign", fmt, ## __VA_ARGS__) the new code should probably try to do similar: #define DEBUG(fmt, ...) \ QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED, \ "pci_assign", fmt, ## __VA_ARGS__) Although __VA_ARGS__ is required by C99, the use of ##__VA_ARGS__ is a gcc extension; are you sure that all other supported compilers handle it? (I guess that's just clang) If you want something portable to C99, just use one fewer macro argument, so that you are guaranteed that __VA_ARGS__ will be non-empty (that is, subsume fmt into the ...): #define DEBUG(...) \ QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED, \ "pci_assign", __VA_ARGS__) > =20 > +#define mb_debug(a...) QEMU_DPRINTF(DEBUG_MULTIBOOT_ENABLED, "i386 mul= tiboot", a) Use of 'a...' as a named var-arg parameter is a gcc extension, not portable to C99. Again, will this compile on non-gcc? Unnamed ... coupled with __VA_ARGS__ is the only fully portable way to do var-args. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --GqBIbKauJXsCjP5OlLTq4LKGIiCs14W5W 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/ iQEcBAEBCAAGBQJTcjcqAAoJEKeha0olJ0NqGZAH/Rubl1jQf8quAgtzBH1az/e5 dafQqOmPf03UGEtIrDctNhmGbT4OwNtiE9bEzG4h96O9mc2LL0009mbHFvJOCVm5 Q1dz5CPmihcpwVgMGSY99vnzotpvCceVa1XGlsa+Q2ELqKMTnIXExUWm+YFFuhCC WkLDjIBm2fCwSlGU0FC0Golw1dBwIwDWJVMbndPnBaLQbBdCqsCDWz2YUw4PhzrD VdHhf5zpg42Wa6Cg3CAZohOlhCw5TzqXNhpqU7YDkdaFw8C735ZQ8JIgYvpKbFhP 6ERK2B/za3SA0sHaTrhBU5/RK9GF5nrgr/Vh0iAFL0HIV9GiRGfRyu19U2PT0Wk= =RVd/ -----END PGP SIGNATURE----- --GqBIbKauJXsCjP5OlLTq4LKGIiCs14W5W--