Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name

[Marc-André Lureau]

Hi

On Thu, Aug 4, 2016 at 5:58 PM Markus Armbruster <address@hidden> wrote:

> address@hidden writes:
>
> > From: Marc-André Lureau <address@hidden>
> >
> > Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
> > class base init name generation instead. Get rid of some leaks that way.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  hw/core/machine.c    | 1 +
> >  include/hw/boards.h  | 2 +-
> >  include/hw/i386/pc.h | 1 -
> >  3 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index e5a456f..00fbe3e 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -561,6 +561,7 @@ static void machine_class_finalize(ObjectClass
> *klass, void *data)
> >      if (mc->compat_props) {
> >          g_array_free(mc->compat_props, true);
> >      }
> > +    g_free(mc->name);
> >  }
> >
> >  void machine_register_compat_props(MachineState *machine)
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 3e69eca..e46a744 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -93,7 +93,7 @@ struct MachineClass {
> >      /*< public >*/
> >
> >      const char *family; /* NULL iff @name identifies a standalone
> machtype */
> > -    const char *name;
> > +    char *name;
> >      const char *alias;
> >      const char *desc;
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index eb1d414..afd025a 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -903,7 +903,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> uint64_t *);
> >      { \
> >          MachineClass *mc = MACHINE_CLASS(oc); \
> >          optsfn(mc); \
> > -        mc->name = namestr; \
> >          mc->init = initfn; \
> >      } \
> >      static const TypeInfo pc_machine_type_##suffix = { \
>
> I guess there are is at least one assignment to mc->name not visible in
> this patch that assigns an allocated string, which leaks before the
> patch.  The commit message seems to provide a clue "class base init name
> generation".  I could probably find it with some effort, but patches
> that take that much work to understand make me grumpy.  Please provide
> another clue :)
>

Sorry, thanks for reminding me to write better commit messages.

git grep 'mc->name ='
hw/core/machine.c:        mc->name = g_strndup(cname,

Is that better:

Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
name generation from machine_class_base_init() instead, and free the
corresponding allocation in machine_class_finalize().
-- 
Marc-André Lureau