Re: [Qemu-devel] [PATCH v3 11/24] error: Use error_reportf_err() where it makes obvious sense

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 12/18/2015 08:35 AM, Markus Armbruster wrote:
>> Done with this Coccinelle semantic patch
>> 
>>     @@
>>     expression FMT, E, S;
>>     expression list ARGS;
>>     @@
>>     -    error_report(FMT, ARGS, error_get_pretty(E));
>>     +    error_reportf_err(E, FMT/*@@@*/, ARGS);
>>     (
>>     -    error_free(E);
>>     |
>>       exit(S);
>>     |
>>       abort();
>>     )
>> 
>> followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping,
>> because I can't figure out how to make Coccinelle transform strings.
>> 
>> We now use the error whole instead of just its message obtained with
>> error_get_pretty().  This avoids suppressing its hint (see commit
>> 50b7b00), but I can't see how the errors touched in this commit could
>> come with hints.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
>> +++ b/arch_init.c
>> @@ -258,9 +258,7 @@ void do_acpitable_option(const QemuOpts *opts)
>>  
>>      acpi_table_add(opts, &err);
>>      if (err) {
>> -        error_report("Wrong acpi table provided: %s",
>> -                     error_get_pretty(err));
>> -        error_free(err);
>> +        error_reportf_err(err, "Wrong acpi table provided: ");
>
> Bikeshedding: should error_reportf_err() automatically add the trailing
> ": " to the prefix, instead of having every caller express it?  Would
> affect 10/24 as well.  But I can't see a strong reason to add the churn
> it would cause for a respin, so I won't insist.

I decided not to add ": " automatically, to make the prefix-ness of the
argument a bit more obvious.

> Reviewed-by: Eric Blake <address@hidden>

Thanks!