Re: [PATCH-for-5.1 v2 48/54] scripts/coccinelle: Use &error_abort in TypeInfo::instance_init()

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

07.04.2020 14:03, Philippe Mathieu-Daudé wrote:
> On 4/7/20 9:07 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 06.04.2020 20:47, Philippe Mathieu-Daudé wrote:
>>> The instance_init() calls are not suppose to fail. Add a
>>> Coccinelle script to use &error_abort instead of ignoring
>>> errors by using a NULL Error*.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>   .../use-error_abort-in-instance_init.cocci    | 52 +++++++++++++++++++
>>>   MAINTAINERS                                   |  1 +
>>>   2 files changed, 53 insertions(+)
>>>   create mode 100644 scripts/coccinelle/use-error_abort-in-instance_init.cocci
>>>
>>> diff --git a/scripts/coccinelle/use-error_abort-in-instance_init.cocci b/scripts/coccinelle/use-error_abort-in-instance_init.cocci
>>> new file mode 100644
>>> index 0000000000..8302d74a0c
>>> --- /dev/null
>>> +++ b/scripts/coccinelle/use-error_abort-in-instance_init.cocci
>>> @@ -0,0 +1,52 @@
>>> +// Use &error_abort in TypeInfo::instance_init()
>>> +//
>>> +// Copyright: (C) 2020 Philippe Mathieu-Daudé
>>> +// This work is licensed under the terms of the GNU GPLv2 or later.
>>> +//
>>> +// spatch \
>>> +//  --macro-file scripts/cocci-macro-file.h --include-headers \
>>> +//  --sp-file scripts/coccinelle/use-error_abort-in-instance_init.cocci \
>>> +//  --keep-comments --in-place
>>> +//
>>> +// Inspired by https://www.mail-archive.com/qemu-devel@nongnu.org/msg692500.html
>>> +// and https://www.mail-archive.com/qemu-devel@nongnu.org/msg693637.html
>>> +
>>> +
>>> +@ has_qapi_error @
>>> +@@
>>> +    #include "qapi/error.h"
>>> +
>>> +
>>> +@ match_instance_init @
>>> +TypeInfo info;
>>> +identifier instance_initfn;
>>> +@@
>>> +    info.instance_init = instance_initfn;
>>> +
>>> +
>>> +@ use_error_abort @
>>> +identifier match_instance_init.instance_initfn;
>>> +identifier func_with_error;
>>> +expression parentobj, propname, childobj, size, type, errp;
>>> +position pos;
>>> +@@
>>> +void instance_initfn(...)
>>> +{
>>> +   <+...
>>> +(
>>> +   object_initialize_child(parentobj, propname,
>>> +                           childobj, size, type,
>>> +                           errp, NULL);

I think, it doesn't actually differs from just object_initialize_child(..., NULL); and you don't need all these metavaraibles

>>> +|
>>> +   func_with_error@pos(...,
>>> +-                           NULL);
>>> ++                           &error_abort);
>>> +)
>>
>>
>> Hmm. I don't follow, what are you trying to achieve by this ( | ) construction? The second pattern (func_with_error...) will be matched anyway, with or without first pattern (object_initialize_child...) matched. And first pattern does nothing.
> 
> Expected behavior :)
> 
> If object_initialize_child() matched:
>    do nothing.
> Else:
>    transform.

Ah, understand, thanks. Checked, it works.

Possibly simpler way is just define func_with_errno identifier like this:

identifier func_with_error != object_initialize_child;

> 
>>
>>
>>> +   ...+>
>>> +}
>>> +
>>> +
>>> +@script:python depends on use_error_abort && !has_qapi_error@
>>> +p << use_error_abort.pos;
>>> +@@
>>> +print('[[manual edit required, %s misses #include "qapi/error.h"]]' % p[0].file)
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 14de2a31dc..ae71a0a4b0 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -2059,6 +2059,7 @@ F: scripts/coccinelle/error-use-after-free.cocci
>>>   F: scripts/coccinelle/error_propagate_null.cocci
>>>   F: scripts/coccinelle/remove_local_err.cocci
>>>   F: scripts/coccinelle/simplify-init-realize-error_propagate.cocci
>>> +F: scripts/coccinelle/use-error_abort-in-instance_init.cocci
>>>   F: scripts/coccinelle/use-error_fatal.cocci
>>>   F: scripts/coccinelle/use-error_propagate-in-realize.cocci
>>>
>>
>>
> 


-- 
Best regards,
Vladimir