Re: [Qemu-devel] [PATCH v2 02/16] s390: Convert conditional compilation of debug printfs to regular ifs

[Cornelia Huck]

On Tue, 13 May 2014 09:02:38 +0200
Marc Marí <address@hidden> wrote:

I'd replace the subject with

"s390: convert debug printfs to QEMU_DPRINTF"

which is more descriptive and has the additional virtue of being
shorter ;)

> 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í <address@hidden>
> ---
>  hw/s390x/s390-virtio-bus.c |    8 ++++----
>  hw/s390x/s390-virtio.c     |    8 ++++----
>  target-s390x/helper.c      |   22 ++++++++++++++--------
>  target-s390x/kvm.c         |    8 ++++----
>  4 files changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 9c71afa..c3a6302 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -38,13 +38,13 @@
>  /* #define DEBUG_S390 */
> 
>  #ifdef DEBUG_S390
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 1
>  #else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> +#define DEBUG_S390_ENABLED 0
>  #endif
> 
> +#define DPRINTF(fmt, ...) QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio 
> bus", fmt, ## __VA_ARGS__)

I think this line is a bit too long.

> +
>  #define VIRTIO_EXT_CODE   0x2603
> 
>  static void virtio_s390_bus_new(VirtioBusState *bus, size_t bus_size,
> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
> index aef2003..133c320 100644
> --- a/hw/s390x/s390-virtio.c
> +++ b/hw/s390x/s390-virtio.c
> @@ -42,13 +42,13 @@
>  //#define DEBUG_S390
> 
>  #ifdef DEBUG_S390
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 1
>  #else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> +#define DEBUG_S390_ENABLED 0
>  #endif
> 
> +#define DPRINTF(fmt, ...) QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio", 
> fmt, ## __VA_ARGS__)

dito

> +
>  #define MAX_BLK_DEVS                    10
>  #define ZIPL_FILENAME                   "s390-zipl.rom"
> 

> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index b7b0edc..5aad3c9 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -41,13 +41,13 @@
>  /* #define DEBUG_KVM */
> 
>  #ifdef DEBUG_KVM
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_KVM_ENABLED 1
>  #else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> +#define DEBUG_KVM_ENABLED 0
>  #endif
> 
> +#define DPRINTF(fmt, ...) QEMU_DPRINTF(DEBUG_KVM_ENABLED, "s390 kvm", fmt, 
> ## __VA_ARGS__)

dito :)

> +
>  #define IPA0_DIAG                       0x8300
>  #define IPA0_SIGP                       0xae00
>  #define IPA0_B2                         0xb200