1. Patchew
  2. QEMU
  3. [PATCH 00/11] More miscellaneous error handling fixes
  4. View patch

[PATCH 03/11] s390x/cpumodel: Fix harmless misuse of visit_check_struct()

Markus Armbruster posted 11 patches 5 years, 6 months ago
Maintainers: Anthony Perard <anthony.perard@citrix.com>, Laurent Vivier <lvivier@redhat.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Peter Maydell <peter.maydell@linaro.org>, Andrzej Zaborowski <balrogg@gmail.com>, Juan Quintela <quintela@redhat.com>, Paul Durrant <paul@xen.org>, Richard Henderson <rth@twiddle.net>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Cornelia Huck <cohuck@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Eric Blake <eblake@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Thomas Huth <thuth@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Fam Zheng <fam@euphon.net>, Xiao Guangrong <xiaoguangrong.eric@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>, David Hildenbrand <david@redhat.com>, Paul Burton <pburton@wavecomp.com>
There is a newer version of this series
[PATCH 03/11] s390x/cpumodel: Fix harmless misuse of visit_check_struct()
Posted by Markus Armbruster 5 years, 6 months ago 
Commit e47970f51d "s390x/cpumodel: Fix query-cpu-model-FOO error API
violations" neglected to change visit_end_struct()'s Error ** argument
along with the others.  If visit_end_struct() failed, we'd take the
success path.  Fortunately, it can't fail here:
qobject_input_check_struct() checks we consumed the whole dictionary,
and to get here, we did.  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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 7c32180269..87a8028ad3 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -524,7 +524,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
             }
         }
         if (!err) {
-            visit_check_struct(visitor, errp);
+            visit_check_struct(visitor, &err);
         }
         visit_end_struct(visitor, NULL);
         visit_free(visitor);
-- 
2.21.1
Re: [PATCH 03/11] s390x/cpumodel: Fix harmless misuse of visit_check_struct()
Posted by David Hildenbrand 5 years, 6 months ago 
On 24.04.20 21:20, Markus Armbruster wrote:
> Commit e47970f51d "s390x/cpumodel: Fix query-cpu-model-FOO error API
> violations" neglected to change visit_end_struct()'s Error ** argument
> along with the others.  If visit_end_struct() failed, we'd take the

s/visit_end_struct/visit_check_struct/ ?

> success path.  Fortunately, it can't fail here:
> qobject_input_check_struct() checks we consumed the whole dictionary,
> and to get here, we did.  Fix it anyway.

AFAIKs, if visit_check_struct() failed, we'd still do the memcopy, but
also report the error. Not nice, not bad.

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

> 
> 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 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 7c32180269..87a8028ad3 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -524,7 +524,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
>              }
>          }
>          if (!err) {
> -            visit_check_struct(visitor, errp);
> +            visit_check_struct(visitor, &err);
>          }
>          visit_end_struct(visitor, NULL);
>          visit_free(visitor);
> 


-- 
Thanks,

David / dhildenb
Re: [PATCH 03/11] s390x/cpumodel: Fix harmless misuse of visit_check_struct()
Posted by Markus Armbruster 5 years, 6 months ago 
David Hildenbrand <david@redhat.com> writes:

> On 24.04.20 21:20, Markus Armbruster wrote:
>> Commit e47970f51d "s390x/cpumodel: Fix query-cpu-model-FOO error API
>> violations" neglected to change visit_end_struct()'s Error ** argument
>> along with the others.  If visit_end_struct() failed, we'd take the
>
> s/visit_end_struct/visit_check_struct/ ?

Will fix.

>> success path.  Fortunately, it can't fail here:
>> qobject_input_check_struct() checks we consumed the whole dictionary,
>> and to get here, we did.  Fix it anyway.
>
> AFAIKs, if visit_check_struct() failed, we'd still do the memcopy, but
> also report the error. Not nice, not bad.
>
> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks!
Re: [PATCH 03/11] s390x/cpumodel: Fix harmless misuse of visit_check_struct()
Posted by Cornelia Huck 5 years, 6 months ago 
On Wed, 29 Apr 2020 07:51:04 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> David Hildenbrand <david@redhat.com> writes:
> 
> > On 24.04.20 21:20, Markus Armbruster wrote:  
> >> Commit e47970f51d "s390x/cpumodel: Fix query-cpu-model-FOO error API
> >> violations" neglected to change visit_end_struct()'s Error ** argument
> >> along with the others.  If visit_end_struct() failed, we'd take the  
> >
> > s/visit_end_struct/visit_check_struct/ ?  
> 
> Will fix.
> 
> >> success path.  Fortunately, it can't fail here:
> >> qobject_input_check_struct() checks we consumed the whole dictionary,
> >> and to get here, we did.  Fix it anyway.  
> >
> > AFAIKs, if visit_check_struct() failed, we'd still do the memcopy, but
> > also report the error. Not nice, not bad.
> >
> > Reviewed-by: David Hildenbrand <david@redhat.com>  
> 
> Thanks!

Will you queue this, or shall I queue it?
Re: [PATCH 03/11] s390x/cpumodel: Fix harmless misuse of visit_check_struct()
Posted by Markus Armbruster 5 years, 6 months ago 
Cornelia Huck <cohuck@redhat.com> writes:

> On Wed, 29 Apr 2020 07:51:04 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> David Hildenbrand <david@redhat.com> writes:
>> 
>> > On 24.04.20 21:20, Markus Armbruster wrote:  
>> >> Commit e47970f51d "s390x/cpumodel: Fix query-cpu-model-FOO error API
>> >> violations" neglected to change visit_end_struct()'s Error ** argument
>> >> along with the others.  If visit_end_struct() failed, we'd take the  
>> >
>> > s/visit_end_struct/visit_check_struct/ ?  
>> 
>> Will fix.
>> 
>> >> success path.  Fortunately, it can't fail here:
>> >> qobject_input_check_struct() checks we consumed the whole dictionary,
>> >> and to get here, we did.  Fix it anyway.  
>> >
>> > AFAIKs, if visit_check_struct() failed, we'd still do the memcopy, but
>> > also report the error. Not nice, not bad.
>> >
>> > Reviewed-by: David Hildenbrand <david@redhat.com>  
>> 
>> Thanks!
>
> Will you queue this, or shall I queue it?

Me taking the complete series through my tree would be easiest for me.
But I can cope with other maintainers picking up bits.
Re: [PATCH 03/11] s390x/cpumodel: Fix harmless misuse of visit_check_struct()
Posted by Cornelia Huck 5 years, 6 months ago 
On Mon, 04 May 2020 17:24:59 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Cornelia Huck <cohuck@redhat.com> writes:
> 
> > On Wed, 29 Apr 2020 07:51:04 +0200
> > Markus Armbruster <armbru@redhat.com> wrote:
> >  
> >> David Hildenbrand <david@redhat.com> writes:
> >>   
> >> > On 24.04.20 21:20, Markus Armbruster wrote:    
> >> >> Commit e47970f51d "s390x/cpumodel: Fix query-cpu-model-FOO error API
> >> >> violations" neglected to change visit_end_struct()'s Error ** argument
> >> >> along with the others.  If visit_end_struct() failed, we'd take the    
> >> >
> >> > s/visit_end_struct/visit_check_struct/ ?    
> >> 
> >> Will fix.
> >>   
> >> >> success path.  Fortunately, it can't fail here:
> >> >> qobject_input_check_struct() checks we consumed the whole dictionary,
> >> >> and to get here, we did.  Fix it anyway.    
> >> >
> >> > AFAIKs, if visit_check_struct() failed, we'd still do the memcopy, but
> >> > also report the error. Not nice, not bad.
> >> >
> >> > Reviewed-by: David Hildenbrand <david@redhat.com>    
> >> 
> >> Thanks!  
> >
> > Will you queue this, or shall I queue it?  
> 
> Me taking the complete series through my tree would be easiest for me.
> But I can cope with other maintainers picking up bits.

In that case, have my

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

and feel free to pick up :)

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.