| Message ID | 1461830481-20165-1-git-send-email-zhoujie2011@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
adding qemu-block On 04/28/2016 02:01 AM, Zhou Jie wrote: > Files with conditional debug statements should ensure that the printf is > always compiled. > This prevents bitrot of the format string of the debug statement. > > Signed-off-by: Zhou Jie <zhoujie2011@cn.fujitsu.com> > --- > +++ b/block/curl.c > @@ -32,14 +32,15 @@ > #include <curl/curl.h> > #include "qemu/cutils.h" > > -// #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 > > -#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) > } > > #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)
diff --git a/block/curl.c b/block/curl.c index 5a8f8b6..6655108 100644 --- a/block/curl.c +++ b/block/curl.c @@ -32,14 +32,15 @@ #include <curl/curl.h> #include "qemu/cutils.h" -// #define DEBUG_CURL +#define DEBUG_CURL 0 // #define DEBUG_VERBOSE -#ifdef DEBUG_CURL -#define DPRINTF(fmt, ...) do { printf(fmt, ## __VA_ARGS__); } while (0) -#else -#define DPRINTF(fmt, ...) do { } while (0) -#endif +#define DPRINTF(fmt, ...) \ + do { \ + if (DEBUG_CURL) { \ + printf(fmt, ## __VA_ARGS__); \ + } \ + } while (0) #if LIBCURL_VERSION_NUM >= 0x071000 /* The multi interface timer callback was introduced in 7.16.0 */ diff --git a/block/sheepdog.c b/block/sheepdog.c index 33e0a33..47cca74 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -293,14 +293,13 @@ static inline size_t count_data_objs(const struct SheepdogInode *inode) } #undef DPRINTF -#ifdef DEBUG_SDOG -#define DPRINTF(fmt, args...) \ - do { \ - fprintf(stdout, "%s %d: " fmt, __func__, __LINE__, ##args); \ +#define DEBUG_SDOG 0 +#define DPRINTF(fmt, args...) \ + do { \ + if (DEBUG_SDOG) { \ + fprintf(stdout, "%s %d: " fmt, __func__, __LINE__, ##args); \ + } \ } while (0) -#else -#define DPRINTF(fmt, args...) -#endif typedef struct SheepdogAIOCB SheepdogAIOCB;
Files with conditional debug statements should ensure that the printf is always compiled. This prevents bitrot of the format string of the debug statement. Signed-off-by: Zhou Jie <zhoujie2011@cn.fujitsu.com> --- block/curl.c | 13 +++++++------ block/sheepdog.c | 13 ++++++------- 2 files changed, 13 insertions(+), 13 deletions(-)