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

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [PATCH-for-5.1 v2 48/54] scripts/coccinelle: Use &error_abort in TypeInfo::instance_init()
Date: Tue, 7 Apr 2020 10:07:19 +0300	[thread overview]
Message-ID: <04663728-06f3-85bf-74b7-45184f90aa0a@virtuozzo.com> (raw)
In-Reply-To: <20200406174743.16956-49-f4bug@amsat.org>

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
>   
> 

-- 
Best regards,
Vladimir