[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of d
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of debug printfs to regular ifs |
Date: |
Mon, 28 Apr 2014 15:16:25 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
Am 28.04.2014 14:41, schrieb Peter Crosthwaite:
> On Mon, Apr 28, 2014 at 10:25 PM, Andreas Färber <address@hidden> wrote:
>> Hi Marc,
>>
>> Am 28.04.2014 10:26, schrieb Marc Marí:
>>> From: Marc Marí <address@hidden>
>>>
>>> Modify debug macros as explained in
>>> https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg03642.html
>>>
>>> Signed-off-by: Marc Marí <address@hidden>
>>> ---
>>> 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
- [Qemu-devel] [PATCH 12/14] target-i386: Convert conditional compilation of debug printfs to regular ifs, (continued)
- [Qemu-devel] [PATCH 12/14] target-i386: Convert conditional compilation of debug printfs to regular ifs, Marc Marí, 2014/04/28
- Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of debug printfs to regular ifs, Michael Tokarev, 2014/04/28
- Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of debug printfs to regular ifs, Kevin Wolf, 2014/04/28
- Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of debug printfs to regular ifs, Michael Tokarev, 2014/04/28
- Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of debug printfs to regular ifs, Peter Crosthwaite, 2014/04/28
- Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of debug printfs to regular ifs, Andreas Färber, 2014/04/28
Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of debug printfs to regular ifs, Marc Marí, 2014/04/28