[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v12 2/8] scripts: Coccinelle script to use ERRP_AUTO_PROPAGAT
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v12 2/8] scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE() |
Date: |
Tue, 07 Jul 2020 22:11:28 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Eric Blake <eblake@redhat.com> writes:
> On 7/7/20 11:50 AM, Markus Armbruster wrote:
>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
>> does corresponding changes in code (look for details in
>> include/qapi/error.h)
>>
>> Usage example:
>> spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>> --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
>> --max-width 80 FILES...
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> scripts/coccinelle/auto-propagated-errp.cocci | 337 ++++++++++++++++++
>> include/qapi/error.h | 3 +
>> MAINTAINERS | 1 +
>> 3 files changed, 341 insertions(+)
>> create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>
> Needs a tweak if we go with ERRP_GUARD. But that's easy.
>
>> +
>> +// Convert special case with goto separately.
>> +// I tried merging this into the following rule the obvious way, but
>> +// it made Coccinelle hang on block.c
>> +//
>> +// Note interesting thing: if we don't do it here, and try to fixup
>> +// "out: }" things later after all transformations (the rule will be
>> +// the same, just without error_propagate() call), coccinelle fails to
>> +// match this "out: }".
>
> "out: }" is not valid C; would referring to "out: ; }" fare any better?
We can try for the next batch.
>> +@ disable optional_qualifier@
>> +identifier rule1.fn, rule1.local_err, out;
>> +symbol errp;
>> +@@
>> +
>> + fn(..., Error ** ____, ...)
>> + {
>> + <...
>> +- goto out;
>> ++ return;
>> + ...>
>> +- out:
>> +- error_propagate(errp, local_err);
>> + }
>> +
>> +// Convert most of local_err related stuff.
>> +//
>> +// Note, that we inherit rule1.fn and rule1.local_err names, not
>> +// objects themselves. We may match something not related to the
>> +// pattern matched by rule1. For example, local_err may be defined with
>> +// the same name in different blocks inside one function, and in one
>> +// block follow the propagation pattern and in other block doesn't.
>> +//
>> +// Note also that errp-cleaning functions
>> +// error_free_errp
>> +// error_report_errp
>> +// error_reportf_errp
>> +// warn_report_errp
>> +// warn_reportf_errp
>> +// are not yet implemented. They must call corresponding Error* -
>> +// freeing function and then set *errp to NULL, to avoid further
>> +// propagation to original errp (consider ERRP_AUTO_PROPAGATE in use).
>> +// For example, error_free_errp may look like this:
>> +//
>> +// void error_free_errp(Error **errp)
>> +// {
>> +// error_free(*errp);
>> +// *errp = NULL;
>> +// }
>
> I guess we can still decide later if we want these additional
> functions, or if they will even help after the number of places we
> have already improved after applying this script as-is and with
> Markus' cleanups in place.
Yes.
> While I won't call myself a Coccinelle expert, it at least looks sane
> enough that I'm comfortable if you add:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
- [PATCH v12 1/8] error: New macro ERRP_AUTO_PROPAGATE(), (continued)
- [PATCH v12 1/8] error: New macro ERRP_AUTO_PROPAGATE(), Markus Armbruster, 2020/07/07
- [PATCH v12 3/8] sd: Use ERRP_AUTO_PROPAGATE(), Markus Armbruster, 2020/07/07
- [PATCH v12 4/8] pflash: Use ERRP_AUTO_PROPAGATE(), Markus Armbruster, 2020/07/07
- [PATCH v12 6/8] virtio-9p: Use ERRP_AUTO_PROPAGATE(), Markus Armbruster, 2020/07/07
- [PATCH v12 2/8] scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE(), Markus Armbruster, 2020/07/07
- [PATCH v12 8/8] xen: Use ERRP_AUTO_PROPAGATE(), Markus Armbruster, 2020/07/07
- [PATCH v12 5/8] fw_cfg: Use ERRP_AUTO_PROPAGATE(), Markus Armbruster, 2020/07/07
- [PATCH v12 7/8] nbd: Use ERRP_AUTO_PROPAGATE(), Markus Armbruster, 2020/07/07
- Re: [PATCH v12 0/8] error: auto propagated local_err part I, Vladimir Sementsov-Ogievskiy, 2020/07/07
- Re: [PATCH v12 0/8] error: auto propagated local_err part I, Eric Blake, 2020/07/07