[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 33/44] error: Avoid unnecessary error_propagate() after er
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 33/44] error: Avoid unnecessary error_propagate() after error_setg() |
Date: |
Fri, 03 Jul 2020 08:53:20 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Eric Blake <eblake@redhat.com> writes:
> On 7/2/20 10:49 AM, Markus Armbruster wrote:
>> Replace
>>
>> error_setg(&err, ...);
>> error_propagate(errp, err);
>>
>> by
>>
>> error_setg(errp, ...);
>>
>
>>
>> Candidates for conversion tracked down with this Coccinelle script:
>>
>> @@
>> identifier err, errp;
>> expression list args;
>> @@
>> - error_setg(&err, args);
>> + error_setg(errp, args);
>> ... when != err
>> error_propagate(errp, err);
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ b/backends/cryptodev.c
>> @@ -158,16 +158,15 @@ cryptodev_backend_set_queues(Object *obj, Visitor *v,
>> const char *name,
>> uint32_t value;
>> if (!visit_type_uint32(v, name, &value, &local_err)) {
>> - goto out;
>> + error_propagate(errp, local_err);
>> + return;
>> }
>
> Looks like this error_propgate is spurious if you just use
> if (!visit_type_uint32(..., errp)) {
>
> Oh - that's not the pattern you flagged, and a later patch then
> addresses it. It might help if the commit message for this patch
> mentions that further cleanups are still forthcoming.
Perhaps:
In some places, the transformation results in obviously unnecessary
error_propagate(). The next few commits will eliminate them.
>> +++ b/backends/hostmem-file.c
>> @@ -114,18 +114,16 @@ static void file_memory_backend_set_align(Object *o,
>> Visitor *v,
>> uint64_t val;
>> if (host_memory_backend_mr_inited(backend)) {
>> - error_setg(&local_err, "cannot change property '%s' of %s",
>> - name, object_get_typename(o));
>> - goto out;
>> + error_setg(errp, "cannot change property '%s' of %s", name,
>> + object_get_typename(o));
>> + return;
>> }
>> if (!visit_type_size(v, name, &val, &local_err)) {
>> - goto out;
>> + error_propagate(errp, local_err);
>> + return;
>> }
>
> Another case where the first 'if' matches the subject of this patch,
> and the second 'if' can avoid local_err but that the change will be
> done in a later patch. And several more later on, but this is how far
> it took me to realize that you intentionally saved them for later.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
- [PATCH v2 16/44] qapi: Make visitor functions taking Error ** return bool, not void, (continued)
- [PATCH v2 16/44] qapi: Make visitor functions taking Error ** return bool, not void, Markus Armbruster, 2020/07/02
- [PATCH v2 30/44] qom: Make functions taking Error ** return bool, not 0/-1, Markus Armbruster, 2020/07/02
- [PATCH v2 07/44] qemu-option: Make uses of find_desc_by_name() more similar, Markus Armbruster, 2020/07/02
- [PATCH v2 18/44] qapi: Use returned bool to check for failure, manual part, Markus Armbruster, 2020/07/02
- [PATCH v2 33/44] error: Avoid unnecessary error_propagate() after error_setg(), Markus Armbruster, 2020/07/02
- Re: [PATCH v2 00/44] Less clumsy error checking, Markus Armbruster, 2020/07/02
- [PATCH v2 35/44] error: Eliminate error_propagate() with Coccinelle, part 2, Markus Armbruster, 2020/07/02
- [PATCH v2 39/44] qapi: Smooth visitor error checking in generated code, Markus Armbruster, 2020/07/02
- [PATCH v2 40/44] qapi: Purge error_propagate() from QAPI core, Markus Armbruster, 2020/07/02
- [PATCH v2 38/44] qapi: Smooth another visitor error checking pattern, Markus Armbruster, 2020/07/02
- Re: [PATCH v2 00/44] Less clumsy error checking, no-reply, 2020/07/02
- Re: [PATCH v2 00/44] Less clumsy error checking, no-reply, 2020/07/02