[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Error handling in cpu_x86_create

From: Jan Kiszka
Subject: Re: [Qemu-devel] Error handling in cpu_x86_create
Date: Fri, 02 Aug 2013 17:39:00 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv: Gecko/20080226 SUSE/ Thunderbird/ Mnenhy/

On 2013-07-30 14:21, Igor Mammedov wrote:
> On Tue, 30 Jul 2013 13:00:40 +0200
> Jan Kiszka <address@hidden> wrote:
>> Hi Igor,
>> just noticed by chance that error handling in cpu_x86_create is likely
>> broken after your changes. Any error after cpu = ...object_new() will
>> not properly release the CPU object again nor report NULL to the caller.
>> That means errors should slip through, no? And cpu_x86_init looks similar.
> Failure of object_new() in cpu_x86_create() is not checked since it should
> never fail.
> As for following error handling caller of cpu_x86_create(..., errp) should
> use errp to check for errors and release failed cpu.
> cpu_x86_init() that calls it has:
> ===
> out:
>     if (error) {
>         fprintf(stderr, "%s\n", error_get_pretty(error));
>         error_free(error);
>         if (cpu != NULL) {
>             object_unref(OBJECT(cpu));
> ===
> similar code-path exists in pc_new_cpu()

Not truly:

    cpu = cpu_x86_create(cpu_model, icc_bridge, errp);
    if (!cpu) {
        return cpu;

    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) {
            cpu = NULL;

That's what made me think the error is supposed to be derived from cpu
== NULL, rather than from errp. So the actual uncleanness is here: errp
should be checked instead of cpu, right?


> end goal is to replace cpu_x86_create() with just object_new(FOO_CPU),
> but that needs to x86 CPU subclasses and properties in place so
> we could remove/isolate legacy stuff to legacy hook.
> Did you hit any particular problem with current code?
>> Jan

Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

reply via email to

[Prev in Thread] Current Thread [Next in Thread]