[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] arm: Use g_new() & friends where that makes obv
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] arm: Use g_new() & friends where that makes obvious sense |
Date: |
Wed, 26 Aug 2015 09:44:26 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 08/25/2015 11:39 AM, Markus Armbruster wrote:
>> g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer,
>> for two reasons. One, it catches multiplication overflowing size_t.
>> Two, it returns T * rather than void *, which lets the compiler catch
>> more type errors.
>>
>> This commit only touches allocations with size arguments of the form
>> sizeof(T).
>>
>> Coccinelle semantic patch:
>>
>> @@
>> type T;
>> @@
>> -g_malloc(sizeof(T))
>> +g_new(T, 1)
>> @@
>> type T;
>
> I find it slightly easier to read coccinelle if there is a blank line
> before every second @@, to call attention to metatype vs. changes
> (coccinelle doesn't care either way).
>
> And it's probably possible to write the coccinelle more succinctly, by
> grouping similar rules together using something like this (although I
> didn't actually test it):
>
> @@
> type T;
> @@
> (
> g_malloc
> |
> g_try_malloc
> |
> g_malloc0
> |
> g_try_malloc0
> )
> -(sizeof(T))
> +(T, 1)
>
> but it doesn't matter in the long run.
I tend to write really stupid semantic patches, because when I try to
write a clever one, I usually relearn I'm not nearly clever enough for
that.
Amazing tool all the same.
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
> Both the coccinelle rule and the resulting conversion look sane.
> Reviewed-by: Eric Blake <address@hidden>
>
>> +++ b/hw/gpio/omap_gpio.c
>> @@ -710,8 +710,8 @@ static int omap2_gpio_init(SysBusDevice *sbd)
>> } else {
>> s->modulecount = 6;
>> }
>> - s->modules = g_malloc0(s->modulecount * sizeof(struct omap2_gpio_s));
>> - s->handler = g_malloc0(s->modulecount * 32 * sizeof(qemu_irq));
>> + s->modules = g_new0(struct omap2_gpio_s, s->modulecount);
>> + s->handler = g_new0(qemu_irq, s->modulecount * 32);
>> qdev_init_gpio_in(dev, omap2_gpio_set, s->modulecount * 32);
>> qdev_init_gpio_out(dev, s->handler, s->modulecount * 32);
>
> Thankfully, the '* 32' here does not overflow even pre-patch, since
> s->modulecount was set to a maximum of 6 in the preceding if/else block.
Thanks!