Re: [Qemu-devel] [PATCH 01/43] acpi: made printf always compile in debug output

[Eric Blake]

On 04/01/2017 08:32 AM, Danil Antonov wrote:
>>From 8127ae68568b3c6876a5ee58a74e5ef0439f548a Mon Sep 17 00:00:00 2001
> From: Danil Antonov <address@hidden>
> Date: Wed, 29 Mar 2017 01:59:37 +0300
> Subject: [PATCH 01/43] acpi: made printf always compile in debug output
> 
> Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.

When sending a 43-patch series, PLEASE be sure to use a 0/43 cover
letter ('git config format.coverletter auto' makes this automatic for
'git send-email'), where all the other mails are in-reply-to the cover
letter.  Otherwise, you are spawning 43 separate top-level threads, and
it gets very hard to follow the series in any sane manner in mail
clients that sort by threading.

Most commit messages use imperative present tense ("do this") instead of
past tense ("did this"); so your subject line might be better as:

acpi: make printf always compile in debug output

> 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 <address@hidden>
> ---
>  hw/acpi/pcihp.c | 15 +++++++++------
>  hw/acpi/piix4.c | 16 +++++++++-------
>  2 files changed, 18 insertions(+), 13 deletions(-)
> 

> 
> -#ifdef DEBUG
> -# define ACPI_PCIHP_DPRINTF(format, ...)     printf(format, ## __VA_ARGS__)
> -#else
> -# define ACPI_PCIHP_DPRINTF(format, ...)     do { } while (0)
> -#endif
> +#ifndef DEBUG
> +#define DEBUG 0
> +#endif
> +
> +#define ACPI_PCIHP_DPRINTF(fmt, ...) do {     \
> +    if (DEBUG) {                              \
> +        fprintf(stderr, fmt, ## __VA_ARGS__); \

You are changing the debug output from stdout to stderr.  While this is
probably a good change, you did not mention it in your commit message as
intentional, so that would be worth adding to the commit message.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature