On Tue, Oct 18, 2011 at 1:13 AM, Stuart Brady <s...@zubnet.me.uk> wrote: > 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)
Cool. Please include the spatch with the commit message. > 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)) IIRC I had also problems with identifiers, I could not get checking identifiers against CODING_STYLE to work.
