Eric Blake <ebl...@redhat.com> writes:
> On 6/24/20 11:43 AM, Markus Armbruster wrote:
>> The previous commit enables conversion of
>>
>> foo(..., &err);
>> if (err) {
>> ...
>> }
>>
>> to
>>
>> if (!foo(..., &err)) {
>> ...
>> }
>>
>> for QemuOpts functions that now return true / false on success /
>> error. Coccinelle script:
>>
>
>>
>> Eliminate error_propagate() that are now unnecessary. Delete @err
>> that are now unused. Tidy up line breaks and whitespace.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>
>> 32 files changed, 70 insertions(+), 192 deletions(-)
>
> Another decent chunk of cleanups.
>
>>
>> diff --git a/block.c b/block.c
>> index 30a72bc4c2..77e85f13db 100644
>> --- a/block.c
>> +++ b/block.c
>
>> @@ -6091,8 +6086,8 @@ void bdrv_img_create(const char *filename, const char
>> *fmt,
>> }
>> if (base_filename) {
>> - qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename,
>> &local_err);
>> - if (local_err) {
>> + if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename,
>> + &local_err)) {
>> error_setg(errp, "Backing file not supported for file format
>> '%s'",
>> fmt);
>
> Pre-existing - it is odd that we collect a message into local_err,
> then write something else into errp; the out: label does
> error_propagate(errp, local_err) which ensures there is no leak but
> discards the original err. You could pass NULL instead. But as it is
> pre-existing, passing NULL should be a separate patch.
PATCH 20.
>> goto out;
>> @@ -6100,8 +6095,7 @@ void bdrv_img_create(const char *filename, const char
>> *fmt,
>> }
>> if (base_fmt) {
>> - qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &local_err);
>> - if (local_err) {
>> + if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt,
>> &local_err)) {
>> error_setg(errp, "Backing file format not supported for file "
>> "format '%s'", fmt);
>
> Ditto.
Likewise.
>> +++ b/qemu-img.c
>> @@ -467,8 +467,8 @@ static int add_old_style_options(const char *fmt,
>> QemuOpts *opts,
>> Error *err = NULL;
>> if (base_filename) {
>> - qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename, &err);
>> - if (err) {
>> + if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename,
>> + &err)) {
>> error_report("Backing file not supported for file format '%s'",
>> fmt);
>> error_free(err);
>> @@ -476,8 +476,7 @@ static int add_old_style_options(const char *fmt,
>> QemuOpts *opts,
>> }
>> }
>> if (base_fmt) {
>> - qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &err);
>> - if (err) {
>> + if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &err)) {
>> error_report("Backing file format not supported for file "
>> "format '%s'", fmt);
>> error_free(err);
>
> Ditto.
PATCH 44.
> But the conversion here is sane.
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
