[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: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 01/14] dma: Convert conditional compilation of debug printfs to regular ifs |
Date: |
Mon, 28 Apr 2014 11:11:27 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 28.04.2014 um 10:26 hat Marc Marí geschrieben:
> 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
> +
> +#define DPRINTF(fmt, ...) \
> + do { \
> + if(DEBUG_I82374) { \
Coding style: There should be a space in "if ("
This seems to be pretty consistent across the whole series. In patch 2,
I also saw a "while(0)" without space. I won't comment on each instance
of this, just check your whole series for these things when you prepare
a v2.
> + fprintf(stderr, "I82374: " fmt, ## __VA_ARGS__); \
> + } \
> + } while (0)
> #define BADF(fmt, ...) \
> do { fprintf(stderr, "i82374 ERROR: " fmt , ## __VA_ARGS__); } while (0)
>
> diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
> index 4490372..ac38d7b 100644
> --- a/hw/dma/i8257.c
> +++ b/hw/dma/i8257.c
> @@ -25,17 +25,27 @@
> #include "hw/isa/isa.h"
> #include "qemu/main-loop.h"
>
> -/* #define DEBUG_DMA */
> +/* #define DEBUG_DMA 1*/
>
> #define dolog(...) fprintf (stderr, "dma: " __VA_ARGS__)
> -#ifdef DEBUG_DMA
> -#define linfo(...) fprintf (stderr, "dma: " __VA_ARGS__)
> -#define ldebug(...) fprintf (stderr, "dma: " __VA_ARGS__)
> -#else
> -#define linfo(...)
> -#define ldebug(...)
> +#ifndef DEBUG_DMA
> +#define DEBUG_DMA 0
> #endif
>
> +#define linfo(...) \
> + do { \
> + if(DEBUG_DMA) { \
> + fprintf(stderr, "dma: " __VA_ARGS__); \
> + } \
> + } while (0)
> +
Trailing whitespace.
> +#define ldebug(...) \
> + do { \
> + if(DEBUG_DMA) { \
> + fprintf(stderr, "dma: " __VA_ARGS__); \
> + } \
> + } while (0)
> +
> struct dma_regs {
> int now[2];
> uint16_t base[2];
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index af26632..217b3f3 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -29,18 +29,23 @@
> /********************************************************/
> /* debug rc4030 */
>
> -//#define DEBUG_RC4030
> +//#define DEBUG_RC4030 1
> //#define DEBUG_RC4030_DMA
Should DEBUG_RC4030_DMA be converted as well? It would probably be even
more useful than the other #defines because it enables a whole block of
code and not just a single printf.
Could be a separate patch (series), though.
> #ifdef DEBUG_RC4030
> -#define DPRINTF(fmt, ...) \
> -do { printf("rc4030: " fmt , ## __VA_ARGS__); } while (0)
> static const char* irq_names[] = { "parallel", "floppy", "sound", "video",
> "network", "scsi", "keyboard", "mouse", "serial0", "serial1" };
> #else
> -#define DPRINTF(fmt, ...)
> +#define DEBUG_RC4030 0
> #endif
>
> +#define DPRINTF(fmt, ...) \
> + do { \
> + if(DEBUG_RC4030) { \
> + printf("rc4030: " fmt, ## __VA_ARGS__); \
> + } \
> + } while (0)
> +
> #define RC4030_ERROR(fmt, ...) \
> do { fprintf(stderr, "rc4030 ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); }
> while (0)
Apart from the style issues and potential extension mentioned above,
the logic looks fine.
Reviewed-by: Kevin Wolf <address@hidden>
- [Qemu-devel] [PATCH 07/14] scsi: Convert conditional compilation of debug printfs to regular ifs, (continued)
- [Qemu-devel] [PATCH 07/14] scsi: Convert conditional compilation of debug printfs to regular ifs, Marc Marí, 2014/04/28
- [Qemu-devel] [PATCH 08/14] sd: Convert conditional compilation of debug printfs to regular ifs, Marc Marí, 2014/04/28
- [Qemu-devel] [PATCH 10/14] target-alpha: Convert conditional compilation of debug printfs to regular ifs, Marc Marí, 2014/04/28
- [Qemu-devel] [PATCH 11/14] slirp: Convert conditional compilation of debug printfs to regular ifs, Marc Marí, 2014/04/28
- [Qemu-devel] [PATCH 13/14] target-s390: Convert conditional compilation of debug printfs to regular ifs, Marc Marí, 2014/04/28
- [Qemu-devel] [PATCH 14/14] qemu: Convert conditional compilation of debug printfs to regular ifs, Marc Marí, 2014/04/28
- [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 <=
- 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