Patchwork target-i386:cpu_x86_init():do clean up work

login
register
mail settings
Submitter liguang
Date Nov. 28, 2012, 6:20 a.m.
Message ID <1354083623-2483-1-git-send-email-lig.fnst@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/202374/
State New
Headers show

Comments

liguang - Nov. 28, 2012, 6:20 a.m.
1.remove unused variable env
2.remove redundant error handling

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 target-i386/helper.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)
Eduardo Habkost - Nov. 29, 2012, 6:34 p.m.
On Wed, Nov 28, 2012 at 02:20:23PM +0800, liguang wrote:
> 1.remove unused variable env

It's not unused. You are removing the line that sets env->cpu_model_str.

> 2.remove redundant error handling
> 
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  target-i386/helper.c |   17 ++++++++---------
>  1 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index bf206cf..5686130 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1243,25 +1243,24 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
>  X86CPU *cpu_x86_init(const char *cpu_model)
>  {
>      X86CPU *cpu;
> -    CPUX86State *env;
>      Error *error = NULL;
>  
>      cpu = X86_CPU(object_new(TYPE_X86_CPU));
> -    env = &cpu->env;
> -    env->cpu_model_str = cpu_model;

Why did you remove this line?

>  
> -    if (cpu_x86_register(cpu, cpu_model) < 0) {
> -        object_delete(OBJECT(cpu));
> -        return NULL;
> -    }
> +    if (cpu_x86_register(cpu, cpu_model) < 0)
> +	  goto error_out;

You are mixing tabs and spaces here, and below:

>  
>      x86_cpu_realize(OBJECT(cpu), &error);
>      if (error) {
>          error_free(error);
> -        object_delete(OBJECT(cpu));
> -        return NULL;
> +		goto error_out;

(here)

>      }
> +
>      return cpu;
> +
> + error_out:
> +	object_delete(OBJECT(cpu));
> +	return NULL;

(and here)

>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> -- 
> 1.7.2.5
>
liguang - Dec. 3, 2012, 1:14 a.m.
在 2012-11-29四的 16:34 -0200,Eduardo Habkost写道:
> On Wed, Nov 28, 2012 at 02:20:23PM +0800, liguang wrote:
> > 1.remove unused variable env
> 
> It's not unused. You are removing the line that sets env->cpu_model_str.
> 
> > 2.remove redundant error handling
> > 
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> >  target-i386/helper.c |   17 ++++++++---------
> >  1 files changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/target-i386/helper.c b/target-i386/helper.c
> > index bf206cf..5686130 100644
> > --- a/target-i386/helper.c
> > +++ b/target-i386/helper.c
> > @@ -1243,25 +1243,24 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
> >  X86CPU *cpu_x86_init(const char *cpu_model)
> >  {
> >      X86CPU *cpu;
> > -    CPUX86State *env;
> >      Error *error = NULL;
> >  
> >      cpu = X86_CPU(object_new(TYPE_X86_CPU));
> > -    env = &cpu->env;
> > -    env->cpu_model_str = cpu_model;
> 
> Why did you remove this line?

OK, I drop this change,
or change like this
'cpu->env->cpu_model_str = cpu_model;'
seems a little ugly.

> 
> >  
> > -    if (cpu_x86_register(cpu, cpu_model) < 0) {
> > -        object_delete(OBJECT(cpu));
> > -        return NULL;
> > -    }
> > +    if (cpu_x86_register(cpu, cpu_model) < 0)
> > +	  goto error_out;
> 
> You are mixing tabs and spaces here, and below:
> 
> >  
> >      x86_cpu_realize(OBJECT(cpu), &error);
> >      if (error) {
> >          error_free(error);
> > -        object_delete(OBJECT(cpu));
> > -        return NULL;
> > +		goto error_out;
> 
> (here)
> 
> >      }
> > +
> >      return cpu;
> > +
> > + error_out:
> > +	object_delete(OBJECT(cpu));
> > +	return NULL;
> 
> (and here)
> 
> >  }
> >  
> >  #if !defined(CONFIG_USER_ONLY)
> > -- 
> > 1.7.2.5
> > 
>

Patch

diff --git a/target-i386/helper.c b/target-i386/helper.c
index bf206cf..5686130 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1243,25 +1243,24 @@  int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
     X86CPU *cpu;
-    CPUX86State *env;
     Error *error = NULL;
 
     cpu = X86_CPU(object_new(TYPE_X86_CPU));
-    env = &cpu->env;
-    env->cpu_model_str = cpu_model;
 
-    if (cpu_x86_register(cpu, cpu_model) < 0) {
-        object_delete(OBJECT(cpu));
-        return NULL;
-    }
+    if (cpu_x86_register(cpu, cpu_model) < 0)
+	  goto error_out;
 
     x86_cpu_realize(OBJECT(cpu), &error);
     if (error) {
         error_free(error);
-        object_delete(OBJECT(cpu));
-        return NULL;
+		goto error_out;
     }
+
     return cpu;
+
+ error_out:
+	object_delete(OBJECT(cpu));
+	return NULL;
 }
 
 #if !defined(CONFIG_USER_ONLY)