Re: [Qemu-devel] [PATCH 01/31] Use error_fatal to simplify obvious fatal errors (again)
Eric Blake writes:
> On 10/8/18 12:30 PM, Markus Armbruster wrote:
>> Add a slight improvement of the Coccinelle semantic patch from commit
>> 07d04a0219b,
>
> which shares the same commit title, but does not actually have a
> semantic patch, but rather defers to the even older 007b065. But I'm
> not too worried about either the duplicated commit title nor the chain
> of references to follow - as this is a no-semantic-change patch rather
> than a bugfix, it's less likely to cause confusion to any downstream
> backport efforts.
I'm changing the reference to 007b06578ab anyway.
>> and use it to clean up. It leaves dead Error * variables
>> behind, cleaned up manually.
>
> Coccinelle can handle that too, if we want to make the .cocci file
> longer (but off-hand, I don't remember the exact semantic patch
> formula to express a variable that is initialized but then never used,
> as a result of applying earlier semantic patches). So the manual
> cleanup for now still seems tractable.
We can track down the recipe when the manual cleanup becomes tiresome.
>>
>> Cc: David Gibson
>> Cc: Alexander Graf
>> Cc: Eric Blake
>> Cc: Paolo Bonzini
>> Signed-off-by: Markus Armbruster
>> ---
>> hw/intc/xics_kvm.c | 7 +--
>> qemu-nbd.c | 6 +-
>> scripts/coccinelle/use-error_fatal.cocci | 20
>> vl.c | 7 +--
>> 4 files changed, 23 insertions(+), 17 deletions(-)
>> create mode 100644 scripts/coccinelle/use-error_fatal.cocci
>>
>
>> +++ b/scripts/coccinelle/use-error_fatal.cocci
>> @@ -0,0 +1,20 @@
>> +@@
>> +type T;
>> +identifier FUN, RET;
>> +expression list ARGS;
>> +expression ERR, EC, FAIL;
>
> The slight improvement from the original git commit log script is the
> addition of FAIL,
>
>> +@@
>> +(
>> +-T RET = FUN(ARGS, );
>> ++T RET = FUN(ARGS, _fatal);
>> +|
>> +-RET = FUN(ARGS, );
>> ++RET = FUN(ARGS, _fatal);
>> +|
>> +-FUN(ARGS, );
>> ++FUN(ARGS, _fatal);
>> +)
>> +-if (FAIL) {
>
> and a check for arbitrary condition FAIL rather than a more specific
> ERR != NULL. Makes sense.
Yes.
>> +-error_report_err(ERR);
>> +-exit(EC);
>> +-}
>
> Reviewed-by: Eric Blake
Thanks!
Re: [Qemu-devel] [PATCH 01/31] Use error_fatal to simplify obvious fatal errors (again)
On Mon, Oct 08, 2018 at 07:30:55PM +0200, Markus Armbruster wrote:
> Add a slight improvement of the Coccinelle semantic patch from commit
> 07d04a0219b, and use it to clean up. It leaves dead Error * variables
> behind, cleaned up manually.
>
> Cc: David Gibson
> Cc: Alexander Graf
> Cc: Eric Blake
> Cc: Paolo Bonzini
> Signed-off-by: Markus Armbruster
ppc part
Acked-by: David Gibson
> ---
> hw/intc/xics_kvm.c | 7 +--
> qemu-nbd.c | 6 +-
> scripts/coccinelle/use-error_fatal.cocci | 20
> vl.c | 7 +--
> 4 files changed, 23 insertions(+), 17 deletions(-)
> create mode 100644 scripts/coccinelle/use-error_fatal.cocci
>
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 30c3769a20..e8fa9a53ae 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -198,17 +198,12 @@ static void ics_get_kvm_state(ICSState *ics)
> {
> uint64_t state;
> int i;
> -Error *local_err = NULL;
>
> for (i = 0; i < ics->nr_irqs; i++) {
> ICSIRQState *irq = >irqs[i];
>
> kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> - i + ics->offset, , false, _err);
> -if (local_err) {
> -error_report_err(local_err);
> -exit(1);
> -}
> + i + ics->offset, , false, _fatal);
>
> irq->server = state & KVM_XICS_DESTINATION_MASK;
> irq->saved_priority = (state >> KVM_XICS_PRIORITY_SHIFT)
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index e76fe3082a..7874bc973c 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -1002,11 +1002,7 @@ int main(int argc, char **argv)
> }
>
> exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags,
> nbd_export_closed,
> - writethrough, NULL, _err);
> -if (!exp) {
> -error_report_err(local_err);
> -exit(EXIT_FAILURE);
> -}
> + writethrough, NULL, _fatal);
> nbd_export_set_name(exp, export_name);
> nbd_export_set_description(exp, export_description);
>
> diff --git a/scripts/coccinelle/use-error_fatal.cocci
> b/scripts/coccinelle/use-error_fatal.cocci
> new file mode 100644
> index 00..10fff0aec4
> --- /dev/null
> +++ b/scripts/coccinelle/use-error_fatal.cocci
> @@ -0,0 +1,20 @@
> +@@
> +type T;
> +identifier FUN, RET;
> +expression list ARGS;
> +expression ERR, EC, FAIL;
> +@@
> +(
> +-T RET = FUN(ARGS, );
> ++T RET = FUN(ARGS, _fatal);
> +|
> +-RET = FUN(ARGS, );
> ++RET = FUN(ARGS, _fatal);
> +|
> +-FUN(ARGS, );
> ++FUN(ARGS, _fatal);
> +)
> +-if (FAIL) {
> +-error_report_err(ERR);
> +-exit(EC);
> +-}
> diff --git a/vl.c b/vl.c
> index a867c9c4d9..9d2b38a31f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2002,15 +2002,10 @@ static void select_vgahw(const char *p)
>
> static void parse_display_qapi(const char *optarg)
> {
> -Error *err = NULL;
> DisplayOptions *opts;
> Visitor *v;
>
> -v = qobject_input_visitor_new_str(optarg, "type", );
> -if (!v) {
> -error_report_err(err);
> -exit(1);
> -}
> +v = qobject_input_visitor_new_str(optarg, "type", _fatal);
>
> visit_type_DisplayOptions(v, NULL, , _fatal);
> QAPI_CLONE_MEMBERS(DisplayOptions, , opts);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH 01/31] Use error_fatal to simplify obvious fatal errors (again)
On 10/8/18 12:30 PM, Markus Armbruster wrote:
Add a slight improvement of the Coccinelle semantic patch from commit
07d04a0219b,
which shares the same commit title, but does not actually have a
semantic patch, but rather defers to the even older 007b065. But I'm
not too worried about either the duplicated commit title nor the chain
of references to follow - as this is a no-semantic-change patch rather
than a bugfix, it's less likely to cause confusion to any downstream
backport efforts.
and use it to clean up. It leaves dead Error * variables
behind, cleaned up manually.
Coccinelle can handle that too, if we want to make the .cocci file
longer (but off-hand, I don't remember the exact semantic patch formula
to express a variable that is initialized but then never used, as a
result of applying earlier semantic patches). So the manual cleanup for
now still seems tractable.
Cc: David Gibson
Cc: Alexander Graf
Cc: Eric Blake
Cc: Paolo Bonzini
Signed-off-by: Markus Armbruster
---
hw/intc/xics_kvm.c | 7 +--
qemu-nbd.c | 6 +-
scripts/coccinelle/use-error_fatal.cocci | 20
vl.c | 7 +--
4 files changed, 23 insertions(+), 17 deletions(-)
create mode 100644 scripts/coccinelle/use-error_fatal.cocci
+++ b/scripts/coccinelle/use-error_fatal.cocci
@@ -0,0 +1,20 @@
+@@
+type T;
+identifier FUN, RET;
+expression list ARGS;
+expression ERR, EC, FAIL;
The slight improvement from the original git commit log script is the
addition of FAIL,
+@@
+(
+-T RET = FUN(ARGS, );
++T RET = FUN(ARGS, _fatal);
+|
+-RET = FUN(ARGS, );
++RET = FUN(ARGS, _fatal);
+|
+-FUN(ARGS, );
++FUN(ARGS, _fatal);
+)
+-if (FAIL) {
and a check for arbitrary condition FAIL rather than a more specific ERR
!= NULL. Makes sense.
+-error_report_err(ERR);
+-exit(EC);
+-}
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
