From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56966) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avocR-00039h-JK for qemu-devel@nongnu.org; Thu, 28 Apr 2016 12:16:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avocQ-0005Jf-Gz for qemu-devel@nongnu.org; Thu, 28 Apr 2016 12:16:47 -0400 References: <1461830481-20165-1-git-send-email-zhoujie2011@cn.fujitsu.com> From: Eric Blake Message-ID: <57223767.30301@redhat.com> Date: Thu, 28 Apr 2016 10:16:39 -0600 MIME-Version: 1.0 In-Reply-To: <1461830481-20165-1-git-send-email-zhoujie2011@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="26e2voCpRP9kObrfA4mOB6obu2ovXIPr0" Subject: Re: [Qemu-devel] [PATCH] block: always compile-check debug prints List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhou Jie , qemu-devel@nongnu.org Cc: jcody@redhat.com, qemu block This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --26e2voCpRP9kObrfA4mOB6obu2ovXIPr0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable adding qemu-block On 04/28/2016 02:01 AM, Zhou Jie wrote: > Files with conditional debug statements should ensure that the printf i= s > always compiled. > This prevents bitrot of the format string of the debug statement. >=20 > Signed-off-by: Zhou Jie > --- > +++ b/block/curl.c > @@ -32,14 +32,15 @@ > #include > #include "qemu/cutils.h" > =20 > -// #define DEBUG_CURL > +#define DEBUG_CURL 0 Other patches doing similar work keep the user-visible witness as defined/undefined, so that you can add -DDEBUG_CURL to the CFLAGS without editing any .c files, and create a secondary witness as the actual conditional. See 8c6597123af for an example, where I did: #ifdef DEBUG_NBD -#define TRACE(msg, ...) do { \ - LOG(msg, ## __VA_ARGS__); \ -} while(0) +#define DEBUG_NBD_PRINT 1 #else -#define TRACE(msg, ...) \ - do { } while (0) +#define DEBUG_NBD_PRINT 0 #endif +#define TRACE(msg, ...) do { \ + if (DEBUG_NBD_PRINT) { \ The way you did it, I cannot add -DDEBUG_CURL (because you blindly redefine it back to 0), but have to edit the .c file. > // #define DEBUG_VERBOSE > =20 > -#ifdef DEBUG_CURL > -#define DPRINTF(fmt, ...) do { printf(fmt, ## __VA_ARGS__); } while (0= ) As long as we're touching this, should we make this print to stderr instead of stdout? > +++ b/block/sheepdog.c > @@ -293,14 +293,13 @@ static inline size_t count_data_objs(const struct= SheepdogInode *inode) > } > =20 > #undef DPRINTF > -#ifdef DEBUG_SDOG > -#define DPRINTF(fmt, args...) \ > - do { \ > - fprintf(stdout, "%s %d: " fmt, __func__, __LINE__, ##args); \ Again, as long as we're touching this, 'fprintf(stdout,' looks stupid. Either make it 'printf(' or make the debug go to stderr. > +#define DEBUG_SDOG 0 Same comment about letting the mere definition of the witness variable be sufficient Thanks for doing this; looking forward to v2 (and/or comments from someone more familiar with whether the block layer should be sending debug comments to stderr instead of stdout) --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --26e2voCpRP9kObrfA4mOB6obu2ovXIPr0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXIjdnAAoJEKeha0olJ0NqgnYIAJQI0jTzprJwKl5fEA6SiDAo skSxwbMCI1Wgg9Cc2Kflas1Dwtou/+YNFmwEEYPjOziJvb+X1CuhhnjJvPdm28Ng VMQm6bbXfmWb9G2HZRr/WvxBPXEdrxAGidc0cjPA5JYK5mxNH3/nqvubeddfQU6D qh/09vpAHjJkq0VBd3Ei7TwF3UrR1yJp8PsV0CNQw7UCDzJXdddcB1q+HjKq9+mT ZH0ORfiG7lTw14L1q9MJ9d8j9WNWUJJgtm6sZJYlp9+npJmDhAMtGQysKVEyxtfu pCp6YBpyWxhf+jyP25C/Yeh+2pcGSzQvUtKoYrlSrOKIb5o0imxp/3n2ymhrmxo= =U9Fz -----END PGP SIGNATURE----- --26e2voCpRP9kObrfA4mOB6obu2ovXIPr0--