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

[Markus Armbruster <armbru@redhat.com>]

Thomas Huth <thuth@redhat.com> writes:

> 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).

Done in v8.

>> 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>

Thanks!