| Message ID | 20200406174743.16956-49-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | various: Fix error-propagation with Coccinelle scripts | expand |
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); > +| > + 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. > + ...+> > +} > + > + > +@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 > >
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); >> +| >> + 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. > > >> + ...+> >> +} >> + >> + >> +@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 >> > >
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 >>> >> >> >
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; 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 >>>> >>> >>> >> > >
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 >>>>> >>>> >>>> >>> >> >>
07.04.2020 20:27, Philippe Mathieu-Daudé wrote: > 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}; New syntax for me, great) > 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)? No, I don't. I can guess something like this @ already_has_include @ @@ #include <qapi/error.h> @ depends on !already_has_include && use_error_abort @ #include ... +#include <qapi/error.h> === or maybe in one rule: @@ ... when != error.h #include ... +#include <qapi/error.h> ... when != error.h (possibly last line is not needed).. what spec says about includes: An #include may be followed by "...", <...> or simply .... With either quotes or angle brackets, it is possible to put a partial path, ending with ..., such as <include/...>, or to put a complete path. A #include with ... matches any include, with either quotes or angle brackets. Partial paths or complete are not allowed in the latter case. Something that is added before an include will be put before the last matching include that is not under an ifdef in the file. Likewise, something that is added after an include will be put after the last matching include that is not under an ifdef in the file. > >>> >>>> >>>>> >>>>> >>>>>> + ...+> >>>>>> +} >>>>>> + >>>>>> + >>>>>> +@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 >>>>>> >>>>> >>>>> >>>> >>> >>> >
On Tue, Apr 7, 2020 at 7:55 PM Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > > 07.04.2020 20:27, Philippe Mathieu-Daudé wrote: > > 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}; > > New syntax for me, great) > > > 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)? > > No, I don't. > > I can guess something like this > > @ already_has_include @ > @@ > > #include <qapi/error.h> > > @ depends on !already_has_include && use_error_abort @ > > #include ... > +#include <qapi/error.h> OMG this works! @ depends on !has_qapi_error && use_error_abort @ @@ #include ... + #include "qapi/error.h" Produces: diff -u -p a/e1000.c b/e1000.c --- a/e1000.c +++ b/e1000.c @@ -39,6 +39,7 @@ #include "e1000x_common.h" #include "trace.h" +#include "qapi/error.h" static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; @@ -1774,7 +1775,7 @@ static void e1000_instance_init(Object * E1000State *n = E1000(obj); device_add_bootindex_property(obj, &n->conf.bootindex, "bootindex", "/ethernet-phy@0", - DEVICE(n), NULL); + DEVICE(n), &error_abort); } > > === > > or maybe in one rule: > > @@ > ... when != error.h > #include ... > +#include <qapi/error.h> > ... when != error.h > > > (possibly last line is not needed).. > > what spec says about includes: > > An #include may be followed by "...", <...> or simply .... With either quotes or angle brackets, it is possible to put a partial path, ending with ..., such as <include/...>, or to put a complete path. A #include with ... matches any include, with either quotes or angle brackets. Partial paths or complete are not allowed in the latter case. Something that is added before an include will be put before the last matching include that is not under an ifdef in the file. Likewise, something that is added after an include will be put after the last matching include that is not under an ifdef in the file. > > > > >>> > >>>> > >>>>> > >>>>> > >>>>>> + ...+> > >>>>>> +} > >>>>>> + > >>>>>> + > >>>>>> +@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 >
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); +| + func_with_error@pos(..., +- NULL); ++ &error_abort); +) + ...+> +} + + +@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
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