From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com
Subject: Re: [Qemu-devel] [PATCH] arm: Use g_new() & friends where that makes obvious sense
Date: Tue, 25 Aug 2015 13:18:17 -0600 [thread overview]
Message-ID: <55DCBF79.7040709@redhat.com> (raw)
In-Reply-To: <1440524394-15640-1-git-send-email-armbru@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2033 bytes --]
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 <armbru@redhat.com>
> ---
Both the coccinelle rule and the resulting conversion look sane.
Reviewed-by: Eric Blake <eblake@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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2015-08-25 19:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-25 17:39 [Qemu-devel] [PATCH] arm: Use g_new() & friends where that makes obvious sense Markus Armbruster
2015-08-25 19:18 ` Eric Blake [this message]
2015-08-26 7:44 ` Markus Armbruster
2015-08-27 16:33 ` Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55DCBF79.7040709@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=peter.crosthwaite@xilinx.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).