| Message ID | 1352756342-13716-6-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
在 2012-11-12一的 19:38 -0200，Eduardo Habkost写道： > Doing error handling on a single place will make it easier to make sure > memory is freed, and that error information is properly printed or > returned to the caller. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target-i386/cpu.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 73b0fa1..69f1204 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1506,17 +1506,20 @@ X86CPU *cpu_x86_init(const char *cpu_model) > env->cpu_model_str = cpu_model; > > if (cpu_x86_register(cpu, cpu_model) < 0) { > - object_delete(OBJECT(cpu)); > - return NULL; > + goto error; > } > > x86_cpu_realize(OBJECT(cpu), &error); > if (error) { > - error_free(error); > - object_delete(OBJECT(cpu)); > - return NULL; > + goto error; > } > return cpu; > +error: seems we should not use lable 'error' as it's the same name with variable 'error' hmm, maybe I should send a patch to clarify. > + object_delete(OBJECT(cpu)); > + if (error) { > + error_free(error); > + } > + return NULL; > } > > #if !defined(CONFIG_USER_ONLY)
On Wed, Nov 28, 2012 at 02:08:01PM +0800, li guang wrote: > 在 2012-11-12一的 19:38 -0200，Eduardo Habkost写道： > > Doing error handling on a single place will make it easier to make sure > > memory is freed, and that error information is properly printed or > > returned to the caller. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > target-i386/cpu.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 73b0fa1..69f1204 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -1506,17 +1506,20 @@ X86CPU *cpu_x86_init(const char *cpu_model) > > env->cpu_model_str = cpu_model; > > > > if (cpu_x86_register(cpu, cpu_model) < 0) { > > - object_delete(OBJECT(cpu)); > > - return NULL; > > + goto error; > > } > > > > x86_cpu_realize(OBJECT(cpu), &error); > > if (error) { > > - error_free(error); > > - object_delete(OBJECT(cpu)); > > - return NULL; > > + goto error; > > } > > return cpu; > > +error: > > seems we should not use lable 'error' as it's > the same name with variable 'error' Do you know any compiler that complains about that? I don't find this to be a problem at all. > > hmm, maybe I should send a patch to clarify. > > > > + object_delete(OBJECT(cpu)); > > + if (error) { > > + error_free(error); > > + } > > + return NULL; > > } > > > > #if !defined(CONFIG_USER_ONLY) > > -- > regards! > li guang > linux kernel team at FNST, china > > thinking with brain but heart > living with heart but brain >
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 73b0fa1..69f1204 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1506,17 +1506,20 @@ X86CPU *cpu_x86_init(const char *cpu_model) env->cpu_model_str = cpu_model; if (cpu_x86_register(cpu, cpu_model) < 0) { - object_delete(OBJECT(cpu)); - return NULL; + goto error; } x86_cpu_realize(OBJECT(cpu), &error); if (error) { - error_free(error); - object_delete(OBJECT(cpu)); - return NULL; + goto error; } return cpu; +error: + object_delete(OBJECT(cpu)); + if (error) { + error_free(error); + } + return NULL; } #if !defined(CONFIG_USER_ONLY)
Doing error handling on a single place will make it easier to make sure memory is freed, and that error information is properly printed or returned to the caller. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)