Re: [Qemu-devel] [PATCH] Wire g_new() and friends to the qemu_malloc() family

[Anthony Liguori]

On 08/21/2011 02:17 AM, Blue Swirl wrote:
> On Sun, Aug 21, 2011 at 3:11 AM, Anthony Liguori<address@hidden>  wrote:
> > On 08/20/2011 01:59 AM, Blue Swirl wrote:
> > > On Fri, Aug 19, 2011 at 3:22 PM, Avi Kivity<address@hidden>    wrote:
> > > > On 08/18/2011 09:54 PM, Peter Maydell wrote:
> > > > > On 18 August 2011 18:48, Avi Kivity<address@hidden>      wrote:
> > > > > >   +static GMemVTable gmemvtable = {
> > > > > >   +    .malloc = qemu_malloc,
> > > > > >   +    .realloc = qemu_realloc,
> > > > > >   +    .free = qemu_free,
> > > > > >   +};
> > > > > >   +
> > > > > >   +/**
> > > > > >   + * qemu_malloc_init: initialize memory management
> > > > > >   + */
> > > > > >   +void qemu_malloc_init(void)
> > > > > >   +{
> > > > > >   +    g_mem_set_vtable(&gmemvtable);
> > > > > >   +}
> > > > >
> > > > > Does this mean you can now safely allocate with g_malloc
> > > > > and free with qemu_free, or is mixing the two APIs like that
> > > > > still a no-no ?
> > > >
> > > > You can, but I'd forbid it.  Mixing layers can only lead to tears later
> > > > on.
> > > >
> > > > Best would be to convert qemu_malloc()s to g_new()s and g_malloc()s to
> > > > reduce confusion.
> > >
> > > Also plain malloc() and friends, except in tracing back end for obvious
> > > reasons.
> >
> > sed script:
> >
> > s:qemu_mallocz\( *\)(:g_malloc0\1\(:g
> > s:qemu_malloc\( *\)(:g_malloc\1\(:g
> > s:qemu_free\( *\)(:g_free\1\(:g
> > s:qemu_strdup\( *\)(:g_strdup\1\(:g
> > s:qemu_strndup\( *\)(:g_strndup\1\(:g
>
> nih--;
>
> Too bad GLib does not provide a function which gives aligned memory,
> then also qemu_memalign() and maybe qemu_vmalloc() could be replaced.

Indeed :-/

> The next step (or merged with this step) should be to replace plain
> libc malloc/free/asprintf/strdup with
> g_malloc/g_free/g_strdup_printf/g_strdup.

There's not a lot of these but they need to be audited individual to 
make sure that the frees correspond to mallocs.

> HACKING should be updated to refer to g_* versions instead of qemu_* functions.

That's included in the series.

> > It takes a little build magic too to make sure everything has access to
> > glib.
> >
> > The patch is way too large to post.  Please speak now if you have an
> > objection otherwise I'll commit in a couple days.
> >
> > http://repo.or.cz/w/qemu/aliguori.git/commit/5cc99cedb46b3916dae8a565d5ffc5fb2f2e9bd6
> >
> > If anyone wants to try it out first.
>
> I didn't test it but looks reasonable.

I've pushed so qemu_malloc is no more.

Regards,

Anthony Liguori