Eric Blake <ebl...@redhat.com> 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 <arm...@redhat.com>
>> ---
>
> Both the coccinelle rule and the resulting conversion look sane.
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
>> +++ 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!
