From 668f62ec621e4e2919fb7d4caa5d805764c5852d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:06:02 +0200 Subject: error: Eliminate error_propagate() with Coccinelle, part 1 When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. Convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... return ... } to if (!foo(..., errp)) { ... ... return ... } where nothing else needs @err. Coccinelle script: @rule1 forall@ identifier fun, err, errp, lbl; expression list args, args2; binary operator op; constant c1, c2; symbol false; @@ if ( ( - fun(args, &err, args2) + fun(args, errp, args2) | - !fun(args, &err, args2) + !fun(args, errp, args2) | - fun(args, &err, args2) op c1 + fun(args, errp, args2) op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; ) } @rule2 forall@ identifier fun, err, errp, lbl; expression list args, args2; expression var; binary operator op; constant c1, c2; symbol false; @@ - var = fun(args, &err, args2); + var = fun(args, errp, args2); ... when != err if ( ( var | !var | var op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; | return var; ) } @depends on rule1 || rule2@ identifier err; @@ - Error *err = NULL; ... when != err Not exactly elegant, I'm afraid. The "when != lbl:" is necessary to avoid transforming if (fun(args, &err)) { goto out } ... out: error_propagate(errp, err); even though other paths to label out still need the error_propagate(). For an actual example, see sclp_realize(). Without the "when strict", Coccinelle transforms vfio_msix_setup(), incorrectly. I don't know what exactly "when strict" does, only that it helps here. The match of return is narrower than what I want, but I can't figure out how to express "return where the operand doesn't use @err". For an example where it's too narrow, see vfio_intx_enable(). Silently fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. Line breaks tidied up manually. One nested declaration of @local_err deleted manually. Preexisting unwanted blank line dropped in hw/riscv/sifive_e.c. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200707160613.848843-35-armbru@redhat.com> --- target/arm/cpu64.c | 12 +++--------- target/arm/monitor.c | 3 +-- target/i386/cpu.c | 20 +++++--------------- target/ppc/compat.c | 4 +--- target/s390x/cpu_models.c | 11 +++-------- target/sparc/cpu.c | 4 +--- 6 files changed, 14 insertions(+), 40 deletions(-) (limited to 'target') diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 343c227c09..15494002d2 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -464,11 +464,9 @@ static void cpu_max_set_sve_max_vq(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { ARMCPU *cpu = ARM_CPU(obj); - Error *err = NULL; uint32_t max_vq; - if (!visit_type_uint32(v, name, &max_vq, &err)) { - error_propagate(errp, err); + if (!visit_type_uint32(v, name, &max_vq, errp)) { return; } @@ -509,11 +507,9 @@ static void cpu_arm_set_sve_vq(Object *obj, Visitor *v, const char *name, { ARMCPU *cpu = ARM_CPU(obj); uint32_t vq = atoi(&name[3]) / 128; - Error *err = NULL; bool value; - if (!visit_type_bool(v, name, &value, &err)) { - error_propagate(errp, err); + if (!visit_type_bool(v, name, &value, errp)) { return; } @@ -544,12 +540,10 @@ static void cpu_arm_set_sve(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { ARMCPU *cpu = ARM_CPU(obj); - Error *err = NULL; bool value; uint64_t t; - if (!visit_type_bool(v, name, &value, &err)) { - error_propagate(errp, err); + if (!visit_type_bool(v, name, &value, errp)) { return; } diff --git a/target/arm/monitor.c b/target/arm/monitor.c index c162c5fb68..ba6e01abd0 100644 --- a/target/arm/monitor.c +++ b/target/arm/monitor.c @@ -174,10 +174,9 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, Error *err = NULL; visitor = qobject_input_visitor_new(model->props); - if (!visit_start_struct(visitor, NULL, NULL, 0, &err)) { + if (!visit_start_struct(visitor, NULL, NULL, 0, errp)) { visit_free(visitor); object_unref(obj); - error_propagate(errp, err); return NULL; } diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 2b60b61ced..6f27f12ec7 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4417,11 +4417,9 @@ static void x86_cpuid_version_set_family(Object *obj, Visitor *v, CPUX86State *env = &cpu->env; const int64_t min = 0; const int64_t max = 0xff + 0xf; - Error *local_err = NULL; int64_t value; - if (!visit_type_int(v, name, &value, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_int(v, name, &value, errp)) { return; } if (value < min || value > max) { @@ -4459,11 +4457,9 @@ static void x86_cpuid_version_set_model(Object *obj, Visitor *v, CPUX86State *env = &cpu->env; const int64_t min = 0; const int64_t max = 0xff; - Error *local_err = NULL; int64_t value; - if (!visit_type_int(v, name, &value, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_int(v, name, &value, errp)) { return; } if (value < min || value > max) { @@ -4496,11 +4492,9 @@ static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v, CPUX86State *env = &cpu->env; const int64_t min = 0; const int64_t max = 0xf; - Error *local_err = NULL; int64_t value; - if (!visit_type_int(v, name, &value, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_int(v, name, &value, errp)) { return; } if (value < min || value > max) { @@ -4600,11 +4594,9 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, const char *name, X86CPU *cpu = X86_CPU(obj); const int64_t min = 0; const int64_t max = INT64_MAX; - Error *local_err = NULL; int64_t value; - if (!visit_type_int(v, name, &value, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_int(v, name, &value, errp)) { return; } if (value < min || value > max) { @@ -6798,7 +6790,6 @@ static void x86_cpu_set_bit_prop(Object *obj, Visitor *v, const char *name, DeviceState *dev = DEVICE(obj); X86CPU *cpu = X86_CPU(obj); BitProperty *fp = opaque; - Error *local_err = NULL; bool value; if (dev->realized) { @@ -6806,8 +6797,7 @@ static void x86_cpu_set_bit_prop(Object *obj, Visitor *v, const char *name, return; } - if (!visit_type_bool(v, name, &value, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_bool(v, name, &value, errp)) { return; } diff --git a/target/ppc/compat.c b/target/ppc/compat.c index 42f87a4bfe..08aede88dc 100644 --- a/target/ppc/compat.c +++ b/target/ppc/compat.c @@ -260,12 +260,10 @@ static void ppc_compat_prop_get(Object *obj, Visitor *v, const char *name, static void ppc_compat_prop_set(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { - Error *local_err = NULL; char *value; uint32_t compat_pvr; - if (!visit_type_str(v, name, &value, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_str(v, name, &value, errp)) { return; } diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index f3ba30e93e..c2af226174 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -510,8 +510,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info, if (qdict) { visitor = qobject_input_visitor_new(info->props); - if (!visit_start_struct(visitor, NULL, NULL, 0, &err)) { - error_propagate(errp, err); + if (!visit_start_struct(visitor, NULL, NULL, 0, errp)) { visit_free(visitor); object_unref(obj); return; @@ -999,7 +998,6 @@ static void get_feature(Object *obj, Visitor *v, const char *name, static void set_feature(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { - Error *err = NULL; S390Feat feat = (S390Feat) opaque; DeviceState *dev = DEVICE(obj); S390CPU *cpu = S390_CPU(obj); @@ -1015,8 +1013,7 @@ static void set_feature(Object *obj, Visitor *v, const char *name, return; } - if (!visit_type_bool(v, name, &value, &err)) { - error_propagate(errp, err); + if (!visit_type_bool(v, name, &value, errp)) { return; } if (value) { @@ -1056,7 +1053,6 @@ static void get_feature_group(Object *obj, Visitor *v, const char *name, static void set_feature_group(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { - Error *err = NULL; S390FeatGroup group = (S390FeatGroup) opaque; const S390FeatGroupDef *def = s390_feat_group_def(group); DeviceState *dev = DEVICE(obj); @@ -1073,8 +1069,7 @@ static void set_feature_group(Object *obj, Visitor *v, const char *name, return; } - if (!visit_type_bool(v, name, &value, &err)) { - error_propagate(errp, err); + if (!visit_type_bool(v, name, &value, errp)) { return; } if (value) { diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c index 4a9257005d..cf21efd85f 100644 --- a/target/sparc/cpu.c +++ b/target/sparc/cpu.c @@ -801,11 +801,9 @@ static void sparc_set_nwindows(Object *obj, Visitor *v, const char *name, const int64_t min = MIN_NWINDOWS; const int64_t max = MAX_NWINDOWS; SPARCCPU *cpu = SPARC_CPU(obj); - Error *err = NULL; int64_t value; - if (!visit_type_int(v, name, &value, &err)) { - error_propagate(errp, err); + if (!visit_type_int(v, name, &value, errp)) { return; } -- cgit v1.2.3-55-g7522