| Message ID | 1399964572-5376-17-git-send-email-marc.mari.barcelo@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 13 May 2014 09:02:52 +0200 Marc Marí <marc.mari.barcelo@gmail.com> wrote: > Modify debug macros to have the same format through the codebase and use regular > ifs instead of ifdef. > > As the debug printf is always put in code, some casting had to be added to avoid > warnings treated as errors at compile time. > > Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com> > --- > include/qemu-common.h | 7 +++++++ > migration-rdma.c | 32 ++++++++++++++------------------ > page_cache.c | 10 ++++++---- > 3 files changed, 27 insertions(+), 22 deletions(-) > > 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); > > #endif > + > +#define QEMU_DPRINTF(cond,pfx,fmt,...) \ > + do { \ > + if (cond) { \ > + fprintf(stderr, pfx": %s:"fmt, __func__, ## __VA_ARGS__); \ > + } \ > + } while (0) I'd split that hunk into a seperate patch and make it the first one in the series. That way, you'd preserve bisectability.
On 05/13/2014 01:02 AM, Marc Marí wrote: > Modify debug macros to have the same format through the codebase and use regular > ifs instead of ifdef. > > As the debug printf is always put in code, some casting had to be added to avoid > warnings treated as errors at compile time. > > Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com> > --- > include/qemu-common.h | 7 +++++++ > migration-rdma.c | 32 ++++++++++++++------------------ > page_cache.c | 10 ++++++---- > 3 files changed, 27 insertions(+), 22 deletions(-) > > 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); > > #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 '...'. 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) { \ ...... 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) == NULL)) { \ > - error_setg(errp, "RDMA ERROR: " fmt, ## __VA_ARGS__); \ > - } \ > - } while (0) > + QEMU_DPRINTF(1, "RDMA ERROR", fmt"\n", ## __VA_ARGS__); \ > + do { if (errp && (*(errp) == NULL)) { error_setg(errp, "RDMA ERROR: " 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) {
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); #endif + +#define QEMU_DPRINTF(cond,pfx,fmt,...) \ + do { \ + if (cond) { \ + fprintf(stderr, pfx": %s:"fmt, __func__, ## __VA_ARGS__); \ + } \ + } while (0) diff --git a/migration-rdma.c b/migration-rdma.c index eeb4302..9f9fa8d 100644 --- a/migration-rdma.c +++ b/migration-rdma.c @@ -32,39 +32,35 @@ //#define DEBUG_RDMA_REALLY_VERBOSE #ifdef DEBUG_RDMA -#define DPRINTF(fmt, ...) \ - do { printf("rdma: " fmt, ## __VA_ARGS__); } while (0) +#define DEBUG_RDMA_ENABLED 1 #else -#define DPRINTF(fmt, ...) \ - do { } while (0) +#define DEBUG_RDMA_ENABLED 0 #endif +#define DPRINTF(fmt, ...) QEMU_DPRINTF(DEBUG_RDMA_ENABLED, "rdma", fmt, ## __VA_ARGS__) + #ifdef DEBUG_RDMA_VERBOSE -#define DDPRINTF(fmt, ...) \ - do { printf("rdma: " fmt, ## __VA_ARGS__); } while (0) +#define DEBUG_RDMA_VERBOSE_ENABLED 1 #else -#define DDPRINTF(fmt, ...) \ - do { } while (0) +#define DEBUG_RDMA_VERBOSE_ENABLED 0 #endif +#define DDPRINTF(fmt, ...) QEMU_DPRINTF(DEBUG_RDMA_VERBOSE_ENABLED, "rdma", fmt, ## __VA_ARGS__) + #ifdef DEBUG_RDMA_REALLY_VERBOSE -#define DDDPRINTF(fmt, ...) \ - do { printf("rdma: " fmt, ## __VA_ARGS__); } while (0) +#define DEBUG_RDMA_REALLY_VERBOSE_ENABLED 1 #else -#define DDDPRINTF(fmt, ...) \ - do { } while (0) +#define DEBUG_RDMA_REALLY_VERBOSE_ENABLED 0 #endif +#define DDDPRINTF(fmt, ...) QEMU_DPRINTF(DEBUG_RDMA_REALLY_VERBOSE_ENABLED, "rdma", fmt, ## __VA_ARGS__) + /* * Print and error on both the Monitor and the Log file. */ #define ERROR(errp, fmt, ...) \ - do { \ - fprintf(stderr, "RDMA ERROR: " fmt "\n", ## __VA_ARGS__); \ - if (errp && (*(errp) == NULL)) { \ - error_setg(errp, "RDMA ERROR: " fmt, ## __VA_ARGS__); \ - } \ - } while (0) + QEMU_DPRINTF(1, "RDMA ERROR", fmt"\n", ## __VA_ARGS__); \ + do { if (errp && (*(errp) == NULL)) { error_setg(errp, "RDMA ERROR: " fmt, ## __VA_ARGS__); } } while (0) #define RDMA_RESOLVE_TIMEOUT_MS 10000 diff --git a/page_cache.c b/page_cache.c index b033681..514d8c0 100644 --- a/page_cache.c +++ b/page_cache.c @@ -25,14 +25,16 @@ #include "qemu-common.h" #include "migration/page_cache.h" +//#define DEBUG_CACHE 1 + #ifdef DEBUG_CACHE -#define DPRINTF(fmt, ...) \ - do { fprintf(stdout, "cache: " fmt, ## __VA_ARGS__); } while (0) +#define DEBUG_CACHE_ENABLED 1 #else -#define DPRINTF(fmt, ...) \ - do { } while (0) +#define DEBUG_CACHE_ENABLED 0 #endif +#define DPRINTF(fmt, ...) QEMU_DPRINTF(DEBUG_CACHE_ENABLED, "cache", fmt, ## __VA_ARGS__) + typedef struct CacheItem CacheItem; struct CacheItem {
Modify debug macros to have the same format through the codebase and use regular ifs instead of ifdef. As the debug printf is always put in code, some casting had to be added to avoid warnings treated as errors at compile time. Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com> --- include/qemu-common.h | 7 +++++++ migration-rdma.c | 32 ++++++++++++++------------------ page_cache.c | 10 ++++++---- 3 files changed, 27 insertions(+), 22 deletions(-)