[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init g
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name |
Date: |
Thu, 04 Aug 2016 17:26:21 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> 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.
Good commit messages are hard.
> git grep 'mc->name ='
> hw/core/machine.c: mc->name = g_strndup(cname,
Aha: the concrete machine type's init function overwrites the strdup()ed
value set by machine_class_base_init(), leaking it. Your fix removes
the overwrites and adds a free.
As far as I can see, you got all such overwrites.
> 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().
Works for me. Alternatively:
machine_class_base_init() member name is allocated by
machine_class_base_init(), but not freed by machine_class_finalize().
Simply freeing there doesn't work, because DEFINE_PC_MACHINE()
overwrites it with a literal string.
Fix DEFINE_PC_MACHINE() not to overwrite it, and add the missing
free to machine_class_finalize().
Use the one you like better, or mix them up to taste.
Reviewed-by: Markus Armbruster <address@hidden>
- [Qemu-devel] [PATCH for-2.7 v3 21/36] tests: fix qom-test leaks, (continued)
- [Qemu-devel] [PATCH for-2.7 v3 21/36] tests: fix qom-test leaks, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 18/36] acpi-build: fix array leak, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 22/36] pc: free i8259, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 16/36] pc: don't leak a20_line, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 19/36] char: free the tcp connection data when closing, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 23/36] pc: keep gsi reference, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 24/36] ahci: free irqs array, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 25/36] sd: free timer, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 26/36] qjson: free str, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 27/36] virtio-input: free config list, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 28/36] ipmi: free extern timer, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 29/36] usb: free USBDevice.strings, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 30/36] usb: free leaking path, marcandre . lureau, 2016/08/03
- [Qemu-devel] [PATCH for-2.7 v3 31/36] bus: simplify name handling, marcandre . lureau, 2016/08/03