Re: [Qemu-devel] [PATCH] gdbstub: Fix memory leak

[Stuart Brady]

On Mon, Oct 17, 2011 at 10:01:25PM +0200, Stefan Weil wrote:
> 
> The patch also slightly cleans the g_malloc0 statement which was
> touched by that change (no type cast, easier code review).
[...]
> -    s = (GDBRegisterState *)g_malloc0(sizeof(GDBRegisterState));
[...]
> +    s = g_malloc0(sizeof(*s));

Is this the preferred style, or should we be using:

    s = g_new0(GDBRegisterState, 1);

We currently seem to have a mix of the two styles.

I kinda prefer using g_new() (or g_new0()) since provided the 'count'
parameter is correct, it can't really be wrong, whereas using sizeof()
has the potential to be buggy, even if it is still trivial to check
that the code is okay.

I guess I feel g_malloc() would be best reserved for those cases where
we're doing something special which might need a little extra care...

BTW, I've just been experimenting with Coccinelle and produced the
following semantic patch:

   @@ type T; @@
   -(T *)g_malloc(sizeof(T))
   +g_new(T, 1)

   @@ type T; @@
   -(T *)g_malloc0(sizeof(T))
   +g_new0(T, 1)

   @@ type T; expression E; @@
   -(T *)g_malloc(sizeof(T) * E)
   +g_new(T, E)

   @@ type T; expression E; @@
   -(T *)g_malloc0(sizeof(T) * E)
   +g_new0(T, E)

I couldn't get the following working:

   // THIS DOES NOT WORK:
   @@ type T; identifier I; @@
   -T I = g_malloc(sizeof(*I))
   +T I = g_new(T, 1)

I expect this won't work either:

   // THIS PROBABLY DOES NOT WORK EITHER:
   @@ type T; identifier I; @@
   -T I = g_new(T, 1)
   +T I = g_malloc(sizeof(*I))

Cheers,
-- 
Stuart