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

2015-12-18 Thread Markus Armbruster
Eric Blake  writes:

> On 12/17/2015 09:49 AM, Markus Armbruster wrote:
>> Done with this Coccinelle semantic patch
>> 
>> @@
>> expression FMT, E;
>> expression list ARGS;
>> @@
>> -error_report(FMT, ARGS, error_get_pretty(E));
>> +error_reportf_err(E, FMT/*@@@*/, ARGS);
>> (
>> -error_free(E);
>> |
>>   exit(S);
>
> Does S have to be declared an expression, for this branch to work
> correctly?  I guess not, since...

Pasto, fixed.

>> |
>>   abort();
>> )
>> 
>> followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping,
>> because I can't figure out how to make Coccinelle transform strings.
>>
>
> Hey - you can already make it do more than I can.
>
>
>> Signed-off-by: Markus Armbruster 
>> ---
>> +++ 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: ");
>>  exit(1);
>
> ...you properly found this spot.
>
>> +++ b/blockdev.c
>> @@ -1583,13 +1583,10 @@ static void
>> internal_snapshot_abort(BlkActionState *common)
>>  }
>>  
>>  if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
>> - error_report("Failed to delete snapshot with id '%s' and name '%s'
>> on "
>> - "device '%s' in abort: %s",
>> - sn->id_str,
>> - sn->name,
>> - bdrv_get_device_name(bs),
>> - error_get_pretty(local_error));
>> -error_free(local_error);
>> +error_reportf_err(local_error,
>> + "Failed to delete snapshot with id '%s' and name '%s' on " "device
>> '%s' in abort: ",
>
> Whoops; line rewrapping touchups missed here.
>
> With that fixed,
> Reviewed-by: Eric Blake 

Thanks!



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

2015-12-17 Thread Eric Blake
On 12/17/2015 09:49 AM, Markus Armbruster wrote:
> Done with this Coccinelle semantic patch
> 
> @@
> expression FMT, E;
> expression list ARGS;
> @@
> -error_report(FMT, ARGS, error_get_pretty(E));
> +error_reportf_err(E, FMT/*@@@*/, ARGS);
> (
> -error_free(E);
> |
>exit(S);

Does S have to be declared an expression, for this branch to work
correctly?  I guess not, since...

> |
>abort();
> )
> 
> followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping,
> because I can't figure out how to make Coccinelle transform strings.
>

Hey - you can already make it do more than I can.


> Signed-off-by: Markus Armbruster 
> ---
> +++ 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: ");
>  exit(1);

...you properly found this spot.

> +++ b/blockdev.c
> @@ -1583,13 +1583,10 @@ static void internal_snapshot_abort(BlkActionState 
> *common)
>  }
>  
>  if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
> -error_report("Failed to delete snapshot with id '%s' and name '%s' 
> on "
> - "device '%s' in abort: %s",
> - sn->id_str,
> - sn->name,
> - bdrv_get_device_name(bs),
> - error_get_pretty(local_error));
> -error_free(local_error);
> +error_reportf_err(local_error,
> +  "Failed to delete snapshot with id '%s' and name 
> '%s' on " "device '%s' in abort: ",

Whoops; line rewrapping touchups missed here.

With that fixed,
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature