Re: [Qemu-devel] [PATCH]target-i386:cpu_x86_init():do clean up work

[li guang]

在 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 <address@hidden>
> > ---
> >  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
> > 
> 

-- 
regards!
li guang                  
linux kernel team at FNST, china

thinking with brain but heart
living with heart but brain