Erich Dollansky wrote this message on Tue, Feb 03, 2015 at 20:01 +0800:> On Mon, 2 Feb 2015 16:33:07 -0800 > John-Mark Gurney <jmg at funkthat.com> wrote: > > > Erich Dollansky wrote this message on Sun, Feb 01, 2015 at 17:51 > > +0800: > > > int can be 64 bits on a amd64 machine. Why is the author of this > > > code so sure that we will never cross the 32 bit boundary? > > > > Per others, int is currently 32bits on all platforms we support... > > > > I guess adding: > > CTASSERT(sizeof(int) <= 4); > > > > would help fix your concern? at least now the expectation is codified > > and if it breaks, the build will break.. > > > of course. Either avoid the run-time problem or get an error message > and things cannot create hidden problem.Ok, I've generated this patch: Index: utils.c ==================================================================--- utils.c (revision 277938) +++ utils.c (working copy) @@ -58,6 +58,7 @@ char *str; * ever convert will be 2^32-1, which is 10 * digits. */ +_Static_assert(sizeof(int) <= 4, "buffer too small for this sized int"); char *itoa(val) I couldn't use CTASSERT because including sys/param.h and sys/systm.h per man page didn't work.. apparently for userland it's different and requires additional headers... It appears that we define it in sys/cdefs.h which gets included, so this work for both clang and gcc... I'll commit it soon unless someone objects... -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."
Hi, On Tue, 3 Feb 2015 23:42:35 -0800 John-Mark Gurney <jmg at funkthat.com> wrote:> Erich Dollansky wrote this message on Tue, Feb 03, 2015 at 20:01 > +0800: > > On Mon, 2 Feb 2015 16:33:07 -0800 > > John-Mark Gurney <jmg at funkthat.com> wrote: > > > > > Erich Dollansky wrote this message on Sun, Feb 01, 2015 at 17:51 > > > +0800: > > > > int can be 64 bits on a amd64 machine. Why is the author of this > > > > code so sure that we will never cross the 32 bit boundary? > > > > > > Per others, int is currently 32bits on all platforms we support... > > > > > > I guess adding: > > > CTASSERT(sizeof(int) <= 4); > > > > > > would help fix your concern? at least now the expectation is > > > codified and if it breaks, the build will break.. > > > > > of course. Either avoid the run-time problem or get an error message > > and things cannot create hidden problem. > > Ok, I've generated this patch: > Index: utils.c > ==================================================================> --- utils.c (revision 277938) > +++ utils.c (working copy) > @@ -58,6 +58,7 @@ char *str; > * ever convert will be 2^32-1, > which is 10 > * digits. > */ > +_Static_assert(sizeof(int) <= 4, "buffer too small for this sized > int"); > char *itoa(val) >Wait, shouldn't it the other way around? It will break when sizeof (int) is greater than 4?> > I couldn't use CTASSERT because including sys/param.h and sys/systm.h > per man page didn't work.. apparently for userland it's different and > requires additional headers... It appears that we define it in > sys/cdefs.h which gets included, so this work for both clang and > gcc... > > I'll commit it soon unless someone objects... >Thanks! Erich