Re: [Qemu-devel] [PATCH] puv3: always compile-check debug printf

["Alex Bennée" <alex.bennee@linaro.org>]

Anishka0107 <rimjhim0107@gmail.com> writes:

>     To prevent bitrot of the format string of the debug statement, files with
>   conditional debug statements should ensure that printf is compiled always,
>   and enclosed within if(0) statements and not in #ifdef.
>
> Signed-off-by: Anishka Gupta <rimjhim0107@gmail.com>
> ---
>  include/hw/unicore32/puv3.h | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/unicore32/puv3.h b/include/hw/unicore32/puv3.h
> index 5a4839f..e268484 100644
> --- a/include/hw/unicore32/puv3.h
> +++ b/include/hw/unicore32/puv3.h
> @@ -41,10 +41,14 @@
>  #define PUV3_IRQS_OST0          (26)
>
>  /* All puv3_*.c use DPRINTF for debug. */
> -#ifdef DEBUG_PUV3
> -#define DPRINTF(fmt, ...) printf("%s: " fmt , __func__, ## __VA_ARGS__)
> -#else
> -#define DPRINTF(fmt, ...) do {} while (0)
> -#endif
> +#define DEBUG_PUV3 0
> +
> +#define DPRINTF(fmt, ...)
> +    if (DEBUG_PUV3) {
> +        fprintf(stderr, "%s: " fmt , __func__, ## __VA_ARGS__)
> +    }
> +    else {
> +        do {} while (0)
> +    }

This is incorrect. It's fine not to have an else leg as the compiler
will dead-code away the if (0) part. However you still need to wrap in a
do {} while to avoid problems with trailing ;'s. Also you need line
continuations for defining macros.

Did you actually compile test this patch? I suspect it wouldn't build
due to the missing ;s for your fprintf and do while.

See hw/misc/imx6_src.c for a debug printf I recently converted.

>
>  #endif /* QEMU_HW_PUV3_H */


--
Alex Bennée