Patchwork [05/17] target-i386: cpu_x86_init(): move error handling to end of function

login
register
mail settings
Submitter Eduardo Habkost
Date Nov. 12, 2012, 9:38 p.m.
Message ID <1352756342-13716-6-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/198495/
State New
Headers show

Comments

Eduardo Habkost - Nov. 12, 2012, 9:38 p.m.
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(-)
liguang - Nov. 28, 2012, 6:08 a.m.
在 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)
Eduardo Habkost - Nov. 28, 2012, 1:21 p.m.
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
>

Patch

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)