| Message ID | 20200904134529.1317-1-pannengyuan@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/i386/cpu: add return value verification and ignore Error objects | expand |
ping! Maybe missed to queue? On 2020/9/4 21:20, Philippe Mathieu-Daudé wrote: > On 9/4/20 3:45 PM, Pan Nengyuan wrote: >> 'err' is unnecessary in x86_cpu_class_check_missing_features(), we can change x86_cpu_expand_features() >> to return true on success, false on failure, then pass NULL here to remove it. >> >> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> >> Suggested-by: Markus Armbruster <armbru@redhat.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> --- >> target/i386/cpu.c | 15 +++++++-------- >> 1 file changed, 7 insertions(+), 8 deletions(-) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 49d8958528..c3d3766133 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -4883,7 +4883,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, >> } >> } >> >> -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp); >> +static bool x86_cpu_expand_features(X86CPU *cpu, Error **errp); >> static void x86_cpu_filter_features(X86CPU *cpu, bool verbose); >> >> /* Build a list with the name of all features on a feature word array */ >> @@ -4925,7 +4925,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, >> strList **missing_feats) >> { >> X86CPU *xc; >> - Error *err = NULL; >> strList **next = missing_feats; >> >> if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) { >> @@ -4937,8 +4936,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, >> >> xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc))); >> >> - x86_cpu_expand_features(xc, &err); >> - if (err) { >> + if (!x86_cpu_expand_features(xc, NULL)) { >> /* Errors at x86_cpu_expand_features should never happen, >> * but in case it does, just report the model as not >> * runnable at all using the "type" property. >> @@ -4947,7 +4945,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, >> new->value = g_strdup("type"); >> *next = new; >> next = &new->next; >> - error_free(err); >> } >> >> x86_cpu_filter_features(xc, false); >> @@ -6426,7 +6423,7 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) >> /* Expand CPU configuration data, based on configured features >> * and host/accelerator capabilities when appropriate. >> */ >> -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) >> +static bool x86_cpu_expand_features(X86CPU *cpu, Error **errp) >> { >> CPUX86State *env = &cpu->env; >> FeatureWord w; >> @@ -6436,14 +6433,14 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) >> for (l = plus_features; l; l = l->next) { >> const char *prop = l->data; >> if (!object_property_set_bool(OBJECT(cpu), prop, true, errp)) { >> - return; >> + return false; >> } >> } >> >> for (l = minus_features; l; l = l->next) { >> const char *prop = l->data; >> if (!object_property_set_bool(OBJECT(cpu), prop, false, errp)) { >> - return; >> + return false; >> } >> } >> >> @@ -6540,6 +6537,8 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) >> if (env->cpuid_xlevel2 == UINT32_MAX) { >> env->cpuid_xlevel2 = env->cpuid_min_xlevel2; >> } >> + >> + return true; >> } >> >> /* >> >
Pan Nengyuan <pannengyuan@huawei.com> 于2020年9月4日周五 下午3:19写道： > > 'err' is unnecessary in x86_cpu_class_check_missing_features(), we can change x86_cpu_expand_features() > to return true on success, false on failure, then pass NULL here to remove it. > > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > Suggested-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Li Qiang <liq3ea@gmail.com> > --- > target/i386/cpu.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 49d8958528..c3d3766133 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -4883,7 +4883,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, > } > } > > -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp); > +static bool x86_cpu_expand_features(X86CPU *cpu, Error **errp); > static void x86_cpu_filter_features(X86CPU *cpu, bool verbose); > > /* Build a list with the name of all features on a feature word array */ > @@ -4925,7 +4925,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, > strList **missing_feats) > { > X86CPU *xc; > - Error *err = NULL; > strList **next = missing_feats; > > if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) { > @@ -4937,8 +4936,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, > > xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc))); > > - x86_cpu_expand_features(xc, &err); > - if (err) { > + if (!x86_cpu_expand_features(xc, NULL)) { > /* Errors at x86_cpu_expand_features should never happen, > * but in case it does, just report the model as not > * runnable at all using the "type" property. > @@ -4947,7 +4945,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, > new->value = g_strdup("type"); > *next = new; > next = &new->next; > - error_free(err); > } > > x86_cpu_filter_features(xc, false); > @@ -6426,7 +6423,7 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) > /* Expand CPU configuration data, based on configured features > * and host/accelerator capabilities when appropriate. > */ > -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) > +static bool x86_cpu_expand_features(X86CPU *cpu, Error **errp) > { > CPUX86State *env = &cpu->env; > FeatureWord w; > @@ -6436,14 +6433,14 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) > for (l = plus_features; l; l = l->next) { > const char *prop = l->data; > if (!object_property_set_bool(OBJECT(cpu), prop, true, errp)) { > - return; > + return false; > } > } > > for (l = minus_features; l; l = l->next) { > const char *prop = l->data; > if (!object_property_set_bool(OBJECT(cpu), prop, false, errp)) { > - return; > + return false; > } > } > > @@ -6540,6 +6537,8 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) > if (env->cpuid_xlevel2 == UINT32_MAX) { > env->cpuid_xlevel2 = env->cpuid_min_xlevel2; > } > + > + return true; > } > > /* > -- > 2.18.2 > >
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 49d8958528..c3d3766133 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4883,7 +4883,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, } } -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp); +static bool x86_cpu_expand_features(X86CPU *cpu, Error **errp); static void x86_cpu_filter_features(X86CPU *cpu, bool verbose); /* Build a list with the name of all features on a feature word array */ @@ -4925,7 +4925,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, strList **missing_feats) { X86CPU *xc; - Error *err = NULL; strList **next = missing_feats; if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) { @@ -4937,8 +4936,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc))); - x86_cpu_expand_features(xc, &err); - if (err) { + if (!x86_cpu_expand_features(xc, NULL)) { /* Errors at x86_cpu_expand_features should never happen, * but in case it does, just report the model as not * runnable at all using the "type" property. @@ -4947,7 +4945,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, new->value = g_strdup("type"); *next = new; next = &new->next; - error_free(err); } x86_cpu_filter_features(xc, false); @@ -6426,7 +6423,7 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) /* Expand CPU configuration data, based on configured features * and host/accelerator capabilities when appropriate. */ -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) +static bool x86_cpu_expand_features(X86CPU *cpu, Error **errp) { CPUX86State *env = &cpu->env; FeatureWord w; @@ -6436,14 +6433,14 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) for (l = plus_features; l; l = l->next) { const char *prop = l->data; if (!object_property_set_bool(OBJECT(cpu), prop, true, errp)) { - return; + return false; } } for (l = minus_features; l; l = l->next) { const char *prop = l->data; if (!object_property_set_bool(OBJECT(cpu), prop, false, errp)) { - return; + return false; } } @@ -6540,6 +6537,8 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) if (env->cpuid_xlevel2 == UINT32_MAX) { env->cpuid_xlevel2 = env->cpuid_min_xlevel2; } + + return true; } /*
'err' is unnecessary in x86_cpu_class_check_missing_features(), we can change x86_cpu_expand_features() to return true on success, false on failure, then pass NULL here to remove it. Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> Suggested-by: Markus Armbruster <armbru@redhat.com> --- target/i386/cpu.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)