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

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 4/7/20 3:07 PM, Philippe Mathieu-Daudé wrote:
> On 4/7/20 3:01 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 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;

Thanks for the tip Vladimir, I simplified as:

@ use_error_abort @
identifier match_instance_init.instance_initfn;
identifier func_with_error != {qbus_create_inplace, 
object_initialize_child};
position pos;
@@
void instance_initfn(...)
{
    <+...
    func_with_error@pos(...,
-                           NULL);
+                           &error_abort);
    ...+>
}

BTW do you know how to automatically add an include ("qapi/error.h" below)?

>>
>>>
>>>>
>>>>
>>>>> +   ...+>
>>>>> +}
>>>>> +
>>>>> +
>>>>> +@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
>>>>>
>>>>
>>>>
>>>
>>
>>