Am 28.04.2014 14:41, schrieb Peter Crosthwaite: > On Mon, Apr 28, 2014 at 10:25 PM, Andreas Färber <afaer...@suse.de> wrote: >> Hi Marc, >> >> Am 28.04.2014 10:26, schrieb Marc Marí: >>> From: Marc Marí <5.markm...@gmail.com> >>> >>> Modify debug macros as explained in >>> https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg03642.html >>> >>> Signed-off-by: Marc Marí <5.markm...@gmail.com> >>> --- >>> hw/dma/i82374.c | 17 ++++++++++------- >>> hw/dma/i8257.c | 24 +++++++++++++++++------- >>> hw/dma/rc4030.c | 13 +++++++++---- >>> 3 files changed, 36 insertions(+), 18 deletions(-) >>> >>> diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c >>> index dc7a767..fff4e6f 100644 >>> --- a/hw/dma/i82374.c >>> +++ b/hw/dma/i82374.c >>> @@ -24,15 +24,18 @@ >>> >>> #include "hw/isa/isa.h" >>> >>> -//#define DEBUG_I82374 >>> +//#define DEBUG_I82374 1 >>> >>> -#ifdef DEBUG_I82374 >>> -#define DPRINTF(fmt, ...) \ >>> -do { fprintf(stderr, "i82374: " fmt , ## __VA_ARGS__); } while (0) >>> -#else >>> -#define DPRINTF(fmt, ...) \ >>> -do {} while (0) >>> +#ifndef DEBUG_I82374 >>> +#define DEBUG_I82374 0 >>> #endif >> >> This is exactly how I told you not to do it in response to Peter C.'s >> proposal. I had done so in my v1 [1] and it was rejected. >> >> Instead it was concluded that we need: >> >> //#define DEBUG_FOO >> >> #ifdef DEBUG_FOO >> #define DEBUG_FOO_ENABLED 1 >> #else >> #define DEBUG_FOO_ENABLED 2
Oops, obviously this what meant to read 0, not 2, i.e. translating absence of a constant to another constant that is assured to always carry a value. true/false may be an alternative for boolean logic. >> #endif >> > > if you are going to go this way you probably want: > > #ifdef DEBUG_FOO > #define DEBUG_FOO_ENABLED DEBUG_FOO > #else > #define DEBUG_FOO_ENABLED 0 > #endif Is it guaranteed that #define DEBUG_FOO => DEBUG_FOO == 1? I assume so. Otherwise this may be just applicable to code where you do this kind of level-based distinction rather than presence/absence. The real question to ask is, does the code have any #ifdef DEBUG_FOO, or does the respective maintainer intend to use it that way? If not, then your if (DEBUG_FOO) {...} is perfectly valid and makes more sense than having ..._ENABLED be anything but a boolean. It's just totally unsafe to assume this to work everywhere in one huge cross-maintainer refactoring series, as my earlier series showed (which did locate and update such #ifdef DEBUG_FOO code). The series becomes non-trivial then. Cheers, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg