1. Patchew
  2. QEMU
  3. View series

[PATCH] target/i386/cpu: add return value verification and ignore Error objects

Pan Nengyuan posted 1 patch 3 years, 7 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200904134529.1317-1-pannengyuan@huawei.com
Maintainers: Richard Henderson <rth@twiddle.net>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>
target/i386/cpu.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
[PATCH] target/i386/cpu: add return value verification and ignore Error objects
Posted by Pan Nengyuan 3 years, 7 months ago 
'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(-)

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
Re: [PATCH] target/i386/cpu: add return value verification and ignore Error objects
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago 
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;
>  }
>  
>  /*
>
Re: [PATCH] target/i386/cpu: add return value verification and ignore Error objects
Posted by Pan Nengyuan 3 years, 6 months ago 
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;
>>  }
>>  
>>  /*
>>
>
Re: [PATCH] target/i386/cpu: add return value verification and ignore Error objects
Posted by Li Qiang 3 years, 6 months ago 
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
>
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.