Re: [Qemu-devel] [PATCH for-2.10 03/10] s390x/pci: make printf always compile in debug output

[Thomas Huth <thuth@redhat.com>]

On 06.04.2017 13:16, Cornelia Huck wrote:
> From: Danil Antonov <g.danil.anto@gmail.com>
> 
> Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
> This will ensure that printf function will always compile even if debug
> output is turned off and, in turn, will prevent bitrot of the format
> strings.
> 
> Signed-off-by: Danil Antonov <g.danil.anto@gmail.com>
> Message-Id: <CA+KKJYBi31Bs7DtVdzZdwG2t+u5+FGiAhQpd3pqJzUX1O8Cprg@mail.gmail.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/s390x/s390-pci-bus.c  | 16 ++++++++++------
>  hw/s390x/s390-pci-inst.c | 16 ++++++++++------
>  2 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 69b0291e8a..0f62363434 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -24,14 +24,18 @@
>  #include "qemu/error-report.h"
>  
>  /* #define DEBUG_S390PCI_BUS */

This comment is now somewhat misleading. If you uncomment this "#define
DEBUG_S390PCI_BUS" (without adding a "1" at the end), you end up with
some empty "if ()" statements now, i.e. compilation failures. So I'd
like to suggest to simply remove the above line.

> -#ifdef DEBUG_S390PCI_BUS
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> +
> +#ifndef DEBUG_S390PCI_BUS
> +#define DEBUG_S390PCI_BUS  0
>  #endif
>  
> +#define DPRINTF(fmt, ...)                                         \
> +    do {                                                          \
> +        if (DEBUG_S390PCI_BUS) {                                  \
> +            fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); \
> +        }                                                         \
> +    } while (0)
> +
>  S390pciState *s390_get_phb(void)
>  {
>      static S390pciState *phb;
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index d2a8c0a083..763eebd67f 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -21,14 +21,18 @@
>  #include "sysemu/hw_accel.h"
>  
>  /* #define DEBUG_S390PCI_INST */

dito

> -#ifdef DEBUG_S390PCI_INST
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, "s390pci-inst: " fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> +
> +#ifndef DEBUG_S390PCI_INST
> +#define DEBUG_S390PCI_INST  0
>  #endif
>  
> +#define DPRINTF(fmt, ...)                                          \
> +    do {                                                           \
> +        if (DEBUG_S390PCI_INST) {                                  \
> +            fprintf(stderr, "s390pci-inst: " fmt, ## __VA_ARGS__); \
> +        }                                                          \
> +    } while (0)
> +
>  static void s390_set_status_code(CPUS390XState *env,
>                                   uint8_t r, uint64_t status_code)
>  {
> 

 Thomas