Message ID | 1375464135-29543-1-git-send-email-afaerber@suse.de |
---|---|
State | New |
Headers | show |
Am 02.08.2013 19:22, schrieb Andreas Färber: > Error **errp argument is not for emitting warnings, it means an error > has occurred and the caller should not make any assumptions about the > state of other return values (unless otherwise documented). > > Therefore cpu_x86_create() must unref the new X86CPU itself, and > pc_new_cpu() must check for an Error rather than NULL return value. > > While at it, clean up a superfluous NULL check. > > Reported-by: Jan Kiszka <jan.kiszka@siemens.com> > Cc: qemu-stable@nongnu.org > Cc: Igor Mammedov <imammedo@redhat.com> > Signed-off-by: Andreas Färber <afaerber@suse.de> Ping! Jan, does this address your concerns / use cases? -rc2 is on Wednesday. Andreas
On 2013-08-05 14:06, Andreas Färber wrote: > Am 02.08.2013 19:22, schrieb Andreas Färber: >> Error **errp argument is not for emitting warnings, it means an error >> has occurred and the caller should not make any assumptions about the >> state of other return values (unless otherwise documented). >> >> Therefore cpu_x86_create() must unref the new X86CPU itself, and >> pc_new_cpu() must check for an Error rather than NULL return value. >> >> While at it, clean up a superfluous NULL check. >> >> Reported-by: Jan Kiszka <jan.kiszka@siemens.com> >> Cc: qemu-stable@nongnu.org >> Cc: Igor Mammedov <imammedo@redhat.com> >> Signed-off-by: Andreas Färber <afaerber@suse.de> > > Ping! Jan, does this address your concerns / use cases? > -rc2 is on Wednesday. Yes, thanks, this looks good. Jan
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a2b9d88..6a0b042 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -912,20 +912,19 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, X86CPU *cpu; Error *local_err = NULL; - cpu = cpu_x86_create(cpu_model, icc_bridge, errp); - if (!cpu) { - return cpu; + cpu = cpu_x86_create(cpu_model, icc_bridge, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return NULL; } object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err); object_property_set_bool(OBJECT(cpu), true, "realized", &local_err); if (local_err) { - if (cpu != NULL) { - object_unref(OBJECT(cpu)); - cpu = NULL; - } error_propagate(errp, local_err); + object_unref(OBJECT(cpu)); + cpu = NULL; } return cpu; } diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 71ab915..2efbeca 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1824,7 +1824,11 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge, } out: - error_propagate(errp, error); + if (error != NULL) { + error_propagate(errp, error); + object_unref(OBJECT(cpu)); + cpu = NULL; + } g_strfreev(model_pieces); return cpu; }
Error **errp argument is not for emitting warnings, it means an error has occurred and the caller should not make any assumptions about the state of other return values (unless otherwise documented). Therefore cpu_x86_create() must unref the new X86CPU itself, and pc_new_cpu() must check for an Error rather than NULL return value. While at it, clean up a superfluous NULL check. Reported-by: Jan Kiszka <jan.kiszka@siemens.com> Cc: qemu-stable@nongnu.org Cc: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de> --- hw/i386/pc.c | 13 ++++++------- target-i386/cpu.c | 6 +++++- 2 files changed, 11 insertions(+), 8 deletions(-)