On Tue, 09 Apr 2013 12:30:21 +0200 Paolo Bonzini <pbonz...@redhat.com> wrote:
> Il 08/04/2013 20:30, Eduardo Habkost ha scritto: > >> > - cpu_x86_register(cpu, name, &error); > >> > - if (error) { > >> > + cpu_x86_register(cpu, name, errp); > >> > + if (error_is_set(errp)) { > > So the function now does error checking properly if and only if errp is > > not NULL. Do we really want to do that? > > No, using error_propagate is the correct idiom indeed. Ok, I'll use error_propagate() in next version. > > Paolo > > >> > goto out; > >> > } > >> > > >> > - cpu_x86_parse_featurestr(cpu, features, &error); > >> > - if (error) { > >> > + cpu_x86_parse_featurestr(cpu, features, errp); > >> > + if (error_is_set(errp)) { > >> > goto out; > >> > } > >> > > >> > - object_property_set_bool(OBJECT(cpu), true, "realized", &error); > >> > +out: > >> > + g_strfreev(model_pieces); > > Any specific reason you didn't choose to keep 'Error *error = NULL' > > inside cpu_x86_create() as well, and use error_propagate() here? I > > believe it would make the patch simpler and easier to review, and at the > > same time make cpu_x86_init() check for errors properly even if errp is > > NULL. This is the opposite of what you did on x86_cpu_realizefn() at > > patch 01/22. >