On 4/14/20 2:24 PM, Markus Armbruster wrote:
Philippe Mathieu-Daudé <f4...@amsat.org> writes:
When a device uses an Error* with data not modified before realize(),
this call can be moved to init(). Add a Coccinelle patch to find such
uses.
Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
---
...implify-init-realize-error_propagate.cocci | 69 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 70 insertions(+)
create mode 100644
scripts/coccinelle/simplify-init-realize-error_propagate.cocci
diff --git a/scripts/coccinelle/simplify-init-realize-error_propagate.cocci
b/scripts/coccinelle/simplify-init-realize-error_propagate.cocci
new file mode 100644
index 0000000000..2e3ec4d98a
--- /dev/null
+++ b/scripts/coccinelle/simplify-init-realize-error_propagate.cocci
@@ -0,0 +1,69 @@
+// Find error-propagation calls that don't need to be in DeviceClass::realize()
+// because they don't use information user can change before calling realize(),
+// so they can be moved to DeviceClass:initfn() where error propagation is not
+// needed.
+//
+// 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 \
+// --sp-file \
+// scripts/coccinelle/simplify-init-realize-error_propagate.cocci \
+// --timeout 60
+//
+// Inspired by
https://www.mail-archive.com/qemu-devel@nongnu.org/msg692500.html
+
+
+@ match_class_init @
+TypeInfo info;
+identifier class_initfn;
+@@
+ info.class_init = class_initfn;
+
+
+@ match_instance_init @
+TypeInfo info;
+identifier instance_initfn;
+@@
+ info.instance_init = instance_initfn;
+
+
+@ match_realize @
+identifier match_class_init.class_initfn;
+DeviceClass *dc;
+identifier realizefn;
+@@
+void class_initfn(...)
+{
+ ...
+ dc->realize = realizefn;
+ ...
+}
I'm afraid this misses realize() methods of DeviceClass subclasses.
Consider PCI device "i6300esb" (picked just because it's simple).
pci_device_class_init() sets DeviceClass method realize() to
pci_qdev_realize(). pci_qdev_realize() does the work common to all PCI
devices, and calls PCIDeviceClass method realize() for the work specific
to the PCI device at hand.
i6300esb_class_init() sets PCIDeviceClass method realize() to
i6300esb_realize().
Your first rule should match i6300esb_info alright, and thus identify
i6300esb_class_init() as a class_init() method.
But your third rule can't match i6300esb_class_init()'s
k->realize = i6300esb_realize;
because @k is a PCIDeviceClass, not a DeviceClass.
I think it also misses cases that have a realize(), but no
instance_init().
Finding only some instances of an anti-pattern can still be useful. But
you should explain the script's limitations then, both in the script and
the commit message.
OK.
+
+
+@ propagate_in_realize @
+identifier match_realize.realizefn;
+identifier err;
+identifier errp;
+identifier func_with_errp =~ "(?!object_property_set_link)";
What are you trying to accomplish with this lookahead assertion?
"match all func_with_errp() except object_property_set_link()"?
+symbol error_abort, error_fatal;
+position pos;
+@@
+void realizefn@pos(..., Error **errp)
+{
+ ...
+ Error *err = NULL;
Why is this line required for a match?
Hmmm maybe I was expecting a local_err... else a left-over from previous
intents :)
+ <+...
+ func_with_errp(..., \(&error_abort\|&error_fatal\));
+ ...+>
+}
+
+
+@ script:python @
+initfn << match_instance_init.instance_initfn;
+realizefn << match_realize.realizefn;
+p << propagate_in_realize.pos;
+@@
+print('>>> possible moves from {}() to {}() in {}:{}'
+ .format(initfn, realizefn, p[0].file, p[0].line))
diff --git a/MAINTAINERS b/MAINTAINERS
index 642c8e0b6b..6203543ec0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2058,6 +2058,7 @@ F: scripts/coccinelle/err-bad-newline.cocci
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_fatal.cocci
GDB stub
