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

[Anthony Liguori <anthony@codemonkey.ws>]

On 08/21/2011 02:17 AM, Blue Swirl wrote:
> On Sun, Aug 21, 2011 at 3:11 AM, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> On 08/20/2011 01:59 AM, Blue Swirl wrote:
>>>
>>> On Fri, Aug 19, 2011 at 3:22 PM, Avi Kivity<avi@redhat.com>    wrote:
>>>>
>>>> On 08/18/2011 09:54 PM, Peter Maydell wrote:
>>>>>
>>>>> On 18 August 2011 18:48, Avi Kivity<avi@redhat.com>      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