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

[Philippe Mathieu-Daudé]

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é <address@hidden>
> > > > ---
> > > >   .../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/address@hidden/msg692500.html
> > > > +// and 
> > > > https://www.mail-archive.com/address@hidden/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;

I didn't know, thanks!

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