Error handling fixes, may contain 4.2 material

[PATCH 18/21] s390x: Fix latent query-cpu-definitions error handling bug

Posted by Markus Armbruster 6 years, 2 months ago

qmp_query_cpu_definitions() tries to ignore get_max_cpu_model()'s
errors.  However, it crashes when its @errp argument is null or
&error_abort, and exit(1)s when it's &error_fatal.  Messed up in
commit 38cba1f4d8 "s390x: return unavailable features via
query-cpu-definitions".

The bug can't bite as no caller actually passes such @errp values.
Fix it anyway.

Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 target/s390x/cpu_models.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 3ed301b5e5..547bab8ac3 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -462,11 +462,7 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
         .list = NULL,
     };
 
-    list_data.model = get_max_cpu_model(errp);
-    if (*errp) {
-        error_free(*errp);
-        *errp = NULL;
-    }
+    list_data.model = get_max_cpu_model(NULL);
 
     object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
                          &list_data);
-- 
2.21.0

Re: [PATCH 18/21] s390x: Fix latent query-cpu-definitions error handling bug

Posted by David Hildenbrand 6 years, 2 months ago

On 30.11.19 20:42, Markus Armbruster wrote:
> qmp_query_cpu_definitions() tries to ignore get_max_cpu_model()'s
> errors.  However, it crashes when its @errp argument is null or
> &error_abort, and exit(1)s when it's &error_fatal.  Messed up in
> commit 38cba1f4d8 "s390x: return unavailable features via
> query-cpu-definitions".

s/crashes .../would crash .../ etc.

Reviewed-by: David Hildenbrand <david@redhat.com>

> 
> The bug can't bite as no caller actually passes such @errp values.
> Fix it anyway.
> 
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  target/s390x/cpu_models.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 3ed301b5e5..547bab8ac3 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -462,11 +462,7 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
>          .list = NULL,
>      };
>  
> -    list_data.model = get_max_cpu_model(errp);
> -    if (*errp) {
> -        error_free(*errp);
> -        *errp = NULL;
> -    }
> +    list_data.model = get_max_cpu_model(NULL);
>  
>      object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
>                           &list_data);
> 


-- 
Thanks,

David / dhildenb