Re: [Qemu-devel] [PATCH v7 10/15] hw/sd: Replace fprintf(stderr, "*\n" with error_report()

[Thomas Huth <thuth@redhat.com>]

On 02.02.2018 19:37, Markus Armbruster wrote:
> From: Alistair Francis <alistair.francis@xilinx.com>
> 
> Replace a large number of the fprintf(stderr, "*\n" calls with
> error_report(). The functions were renamed with these commands and then
> compiler issues where manually fixed.
> 
> find ./* -type f -exec sed -i \
>     'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> find ./* -type f -exec sed -i \
>     'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>     {} +
> 
> Some lines where then manually tweaked to pass checkpatch.

I think you could pretty much ditch the original patch description since
nothing gets converted to error_report here anymore...

> Some of the prints in hw/sd/sd.c were manually converted to using
> DPRINTF() instead.

... and just use the above sentence (in a maybe slightly adapted way).

> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> 
> Conversions that aren't followed by exit() dropped, because they might
> be inappropriate.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/sd/sd.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 35347a5bbc..73e405a04f 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1564,9 +1564,10 @@ send_response:
>      if (rsplen) {
>          int i;
>          DPRINTF("Response:");
> -        for (i = 0; i < rsplen; i++)
> -            fprintf(stderr, " %02x", response[i]);
> -        fprintf(stderr, " state %d\n", sd->state);
> +        for (i = 0; i < rsplen; i++) {
> +            DPRINTF(" %02x", response[i]);
> +        }
> +        DPRINTF(" state %d\n", sd->state);
>      } else {
>          DPRINTF("No response %d\n", sd->state);
>      }
> 

With the patch description updated:

Reviewed-by: Thomas Huth <thuth@redhat.com>