GNOME Bugzilla – Bug 608196
Overflow-safe g_new family
Last modified: 2010-03-08 05:51:08 UTC
Patch coming.
Created attachment 152361 [details] [review] Patch
Comment on attachment 152361 [details] [review] Patch >+ register gsize __n = (gsize) (n_blocks); \ ^^^^^^^^ you are a silly person
(In reply to comment #2) > (From update of attachment 152361 [details] [review]) > >+ register gsize __n = (gsize) (n_blocks); \ > ^^^^^^^^ > you are a silly person Blame copy/paste. Oh, and with that measure, glibc maintainers are the silliest ever.
Looks fine to me in general. Of course, the register should go, and it needs documentation for the new api, and ideally a test case to verify that this works as intended.
Created attachment 152881 [details] [review] Updated patch Moving the macro to g_malloc_n family instead of g_new. Please comment.
Ok, lets try to get this in. We should request API freeze break request. I work on a complete patch.
Created attachment 154567 [details] [review] Complete patch Ok, this one should be ready to go in.
Looks good to me too. +#define CHECK_PASS(P) p = (P); g_assert (p == NULL); +#define CHECK_FAIL(P) p = (P); g_assert (p != NULL); might be more obvious to call these CHECK_NULL and CHECK_NONNULL, maybe
(In reply to comment #9) > Looks good to me too. > > +#define CHECK_PASS(P) p = (P); g_assert (p == NULL); > +#define CHECK_FAIL(P) p = (P); g_assert (p != NULL); > > might be more obvious to call these CHECK_NULL and CHECK_NONNULL, maybe Initially I did, but then for the forking ones that name didn't work. So I opted for these instead. I'm writing freeze break request and will commit after.
Committed. Note that apps need to be rebuilt against new glib to get the protection.
Hi, I'm getting a bunch of redefinition errors after this commit (343cbf25c7104f782b9d0070cb623c7605dab646), In file included from gmem.c:1174: galiasdef.c:1756: error: redefinition of 'g_malloc_n' gmem.c:239: error: previous definition of 'g_malloc_n' was here galiasdef.c:1759: error: redefinition of 'g_malloc0_n' gmem.c:256: error: previous definition of 'g_malloc0_n' was here galiasdef.c:1774: error: redefinition of 'g_realloc_n' gmem.c:274: error: previous definition of 'g_realloc_n' was here galiasdef.c:1783: error: redefinition of 'g_try_malloc_n' gmem.c:291: error: previous definition of 'g_try_malloc_n' was here galiasdef.c:1786: error: redefinition of 'g_try_malloc0_n' gmem.c:302: error: previous definition of 'g_try_malloc0_n' was here galiasdef.c:1792: error: redefinition of 'g_try_realloc_n' gmem.c:314: error: previous definition of 'g_try_realloc_n' was here
Same here.
Same here, and on the build.gnome.org: http://build.gnome.org/glib
Ugh. Fixing.
Should be fixed.
The patch is still causing problems. Reopening. Long story short: it's not possible to have functions and macros with the same name. My suggestion: put the optimisation logic directly into the g_new() macros and have the g_malloc_n() API be pure function calls.
At Behdad's request I went ahead and committed the fix.
Sure it is possible. The C standard explicitly allows this for all standard functions.
(In reply to comment #19) > Sure it is possible. The C standard explicitly allows this for all standard > functions. Sure. The problem is with galias. What was happening was that the g_malloc_n macro and the galias stuff were both fighting for defining g_malloc_n as a macro to implement an optimization. One had to go.