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

From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Michael Roth" <mdroth@linux.vnet.ibm.com>,
	qemu-block@nongnu.org, "Paul Durrant" <paul@xen.org>,
	"Laszlo Ersek" <lersek@redhat.com>,
	"Cornelia Huck" <cohuck@redhat.com>, "Greg Kurz" <groug@kaod.org>,
	"Max Reitz" <mreitz@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	xen-devel@lists.xenproject.org,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Stefan Berger" <stefanb@linux.ibm.com>
Subject: Re: [Xen-devel] [PATCH v6 03/11] scripts: add coccinelle script to use auto propagated errp
Date: Fri, 17 Jan 2020 08:24:55 -0600	[thread overview]
Message-ID: <4e41cb43-4efb-9cc9-c685-d25e4ddf8fbb@redhat.com> (raw)
In-Reply-To: <20200110194158.14190-4-vsementsov@virtuozzo.com>

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

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@redhat.com>

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

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