Re: [Qemu-devel] [PATCH] arm: Use g_new() & friends where that makes obvious sense

[Eric Blake]

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.

> 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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature