[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6 03/11] scripts: add coccinelle script to use auto propagated errp



On 1/10/20 1:41 PM, Vladimir Sementsov-Ogievskiy wrote:
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx>
---

Rather light on the commit message. If nothing else, a comment about typical command-line usage would be helpful (yes, it's in the patch body, but sometimes I just refer to git log).

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 532b9afb9e..dcfb77e107 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -141,6 +141,9 @@
   *         ...
   *     }
   *
+ * For mass conversion use script
+ *   scripts/coccinelle/auto-propagated-errp.cocci
+ *
   *
   * Receive and accumulate multiple errors (first one wins):
   *     Error *err = NULL, *local_err = NULL;
diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 0000000000..6c72a5049f
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci
@@ -0,0 +1,139 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//

+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]
+
+@@
+// Add invocation to errp-functions where necessary
+// We should skip functions with "Error *const *errp"
+// parameter, but how to do it with coccinelle?
+// I don't know, so, I skip them by function name regex.
+// It's safe: if we not skip some functions with

s/not/did not/

+// "Error *const *errp", ERRP_AUTO_PROPAGATE invocation
+// will fail to compile, because of const violation.
+identifier fn !~ "error_append_.*_hint";
+identifier local_err, errp;
+@@
+
+ fn(..., Error **errp, ...)
+ {
++   ERRP_AUTO_PROPAGATE();
+    <+...
+        when != ERRP_AUTO_PROPAGATE();
+(
+    error_append_hint(errp, ...);
+|
+    error_prepend(errp, ...);
+|
+    Error *local_err = NULL;
+)
+    ...+>
+ }
+

Looks like it should catch all functions that require adding the macro.

+@rule1@
+// We do not inherit from previous rule, as we want to match
+// also functions, which already had ERRP_AUTO_PROPAGATE
+// invocation.

Grammar suggestion:

// We want to patch error propagation in functions regardless of
// whether the function already uses ERRP_AUTO_PROPAGATE, hence
// this one does not inherit from the first rule.

Reviewed-by: Eric Blake <eblake@xxxxxxxxxx>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.