Re: [HACKERS] Missing SIZE_MAX
I wrote: > Robert Haas writes: >> It might be worth the effort to clean all of this up, just because the >> next person who gets bitten by it may not be as smart as you are. > Yeah. I was just thinking that maybe the appropriate investment of > effort is to make [U]INT64CONST smarter, so that it results in a > properly-suffixed constant and doesn't need a cast. Then it'd be a > lot easier to make these other macros be #if-safe. Actually, that looks easier than I thought. The current approach to [U]INT64CONST dates to before we were willing to require the compiler to have working 64-bit support. I think that now we can just assume that either an L/UL or LL/ULL suffix will work, as in the attached draft. (This'd allow dropping configure's HAVE_LL_CONSTANTS probe entirely, but I didn't do that yet.) regards, tom lane diff --git a/src/include/c.h b/src/include/c.h index 4f8bbfc..4fb8ef0 100644 *** a/src/include/c.h --- b/src/include/c.h *** typedef long int int64; *** 288,293 --- 288,295 #ifndef HAVE_UINT64 typedef unsigned long int uint64; #endif + #define INT64CONST(x) (x##L) + #define UINT64CONST(x) (x##UL) #elif defined(HAVE_LONG_LONG_INT_64) /* We have working support for "long long int", use that */ *** typedef long long int int64; *** 297,316 #ifndef HAVE_UINT64 typedef unsigned long long int uint64; #endif #else /* neither HAVE_LONG_INT_64 nor HAVE_LONG_LONG_INT_64 */ #error must have a working 64-bit integer datatype #endif - /* Decide if we need to decorate 64-bit constants */ - #ifdef HAVE_LL_CONSTANTS - #define INT64CONST(x) ((int64) x##LL) - #define UINT64CONST(x) ((uint64) x##ULL) - #else - #define INT64CONST(x) ((int64) x) - #define UINT64CONST(x) ((uint64) x) - #endif - /* snprintf format strings to use for 64-bit integers */ #define INT64_FORMAT "%" INT64_MODIFIER "d" #define UINT64_FORMAT "%" INT64_MODIFIER "u" --- 299,311 #ifndef HAVE_UINT64 typedef unsigned long long int uint64; #endif + #define INT64CONST(x) (x##LL) + #define UINT64CONST(x) (x##ULL) #else /* neither HAVE_LONG_INT_64 nor HAVE_LONG_LONG_INT_64 */ #error must have a working 64-bit integer datatype #endif /* snprintf format strings to use for 64-bit integers */ #define INT64_FORMAT "%" INT64_MODIFIER "d" #define UINT64_FORMAT "%" INT64_MODIFIER "u" *** typedef unsigned PG_INT128_TYPE uint128; *** 338,351 #define PG_UINT16_MAX (0x) #define PG_INT32_MIN (-0x7FFF-1) #define PG_INT32_MAX (0x7FFF) ! #define PG_UINT32_MAX (0x) #define PG_INT64_MIN (-INT64CONST(0x7FFF) - 1) #define PG_INT64_MAX INT64CONST(0x7FFF) #define PG_UINT64_MAX UINT64CONST(0x) /* Max value of size_t might also be missing if we don't have stdint.h */ #ifndef SIZE_MAX ! #define SIZE_MAX ((size_t) -1) #endif /* --- 333,350 #define PG_UINT16_MAX (0x) #define PG_INT32_MIN (-0x7FFF-1) #define PG_INT32_MAX (0x7FFF) ! #define PG_UINT32_MAX (0xU) #define PG_INT64_MIN (-INT64CONST(0x7FFF) - 1) #define PG_INT64_MAX INT64CONST(0x7FFF) #define PG_UINT64_MAX UINT64CONST(0x) /* Max value of size_t might also be missing if we don't have stdint.h */ #ifndef SIZE_MAX ! #if SIZEOF_SIZE_T == 8 ! #define SIZE_MAX PG_UINT64_MAX ! #else ! #define SIZE_MAX PG_UINT32_MAX ! #endif #endif /* -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing SIZE_MAX
Robert Haas writes: > On Fri, Sep 1, 2017 at 12:22 PM, Tom Lane wrote: >> [ warning: more than you really wanted to know ahead ] > It might be worth the effort to clean all of this up, just because the > next person who gets bitten by it may not be as smart as you are. Yeah. I was just thinking that maybe the appropriate investment of effort is to make [U]INT64CONST smarter, so that it results in a properly-suffixed constant and doesn't need a cast. Then it'd be a lot easier to make these other macros be #if-safe. Or we could just recast the test in fe-exec.c to not use SIZE_MAX. Checking whether "SIZEOF_SIZE_T == 4" would really have the same effect, though it's uglier. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing SIZE_MAX
On Fri, Sep 1, 2017 at 12:22 PM, Tom Lane wrote: > [ warning: more than you really wanted to know ahead ] It might be worth the effort to clean all of this up, just because the next person who gets bitten by it may not be as smart as you are. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing SIZE_MAX
[ warning: more than you really wanted to know ahead ] Alvaro Herrera writes: > Tom Lane wrote: >> We have a workaround for that symbol in timezone/private.h: >> #ifndef SIZE_MAX >> #define SIZE_MAX ((size_t) -1) >> #endif >> and a bit of grepping finds other places that are using the (size_t) -1 >> trick explicitly. So what I'm tempted to do is move the above stanza >> into c.h. > Sounds good to me. On closer inspection, C99 requires SIZE_MAX and related macros to be a "constant expression suitable for use in #if preprocessing directives", which we need for the fe-exec.c usage because it does #if INT_MAX >= (SIZE_MAX / 2) if (newSize > SIZE_MAX / sizeof(PGresAttValue *)) (We could maybe dispense with this #if check, but I feared that doing so would result in nanny-ish "expression is constant false" warnings from some compilers on 64-bit platforms.) Now, that cast doesn't really work in an #if expression. Some language lawyering leads me to conclude that in #if, a C compiler will interpret the above value of SIZE_MAX as "((0) -1)", or just signed -1. So fe-exec.c's test will surely evaluate to true, which seems like a safe outcome. But you could certainly imagine other cases where you get incorrect results if SIZE_MAX looks like a signed -1 to an #if-test. When I look into /usr/include/stdint.h on my Linux box, I find # if __WORDSIZE == 64 # define SIZE_MAX (18446744073709551615UL) # else # define SIZE_MAX (4294967295U) # endif so I thought about trying to duplicate that logic. We can certainly test SIZEOF_SIZE_T == 8 as a substitute for the #if condition. The hard part is arriving at a portable spelling of "UL", since it would need to be "ULL" instead on some platforms. We can't make use of our UINT64CONST macro because that includes a cast. So it seems like if we want to be 100% correct it would need to be something like #ifndef SIZE_MAX #if SIZEOF_SIZE_T == 8 #if SIZEOF_LONG == 8 #define SIZE_MAX (18446744073709551615UL) #else /* assume unsigned long long is what we need */ #define SIZE_MAX (18446744073709551615ULL) #endif #else /* 32-bit */ #define SIZE_MAX (4294967295U) #endif #endif That's mighty ugly. Is it worth the trouble, rather than trusting that the "(size_t) -1" trick will work? Given that we have so few needs for SIZE_MAX, I'm kind of inclined just to stick with the cast. I notice BTW that PG_INT64_MIN, PG_INT64_MAX, and PG_UINT64_MAX all contain casts and thus are equally risky to test in #if-tests. I see no at-risk code in our tree right now, but someday we might need to make those look something like the above, too. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing SIZE_MAX
Tom Lane wrote: > We have a workaround for that symbol in timezone/private.h: > > #ifndef SIZE_MAX > #define SIZE_MAX ((size_t) -1) > #endif > > and a bit of grepping finds other places that are using the (size_t) -1 > trick explicitly. So what I'm tempted to do is move the above stanza > into c.h. Sounds good to me. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing SIZE_MAX
On Fri, Sep 1, 2017 at 11:09 AM, Tom Lane wrote: > Commit 2e70d6b5e added a dependency on SIZE_MAX to libpq/fe_exec.c. > According to C99 and recent POSIX, that symbol should be provided > by , but SUS v2 (POSIX 2001) doesn't require > to exist at all ... and I now notice that gaur/pademelon doesn't > have it, and unsurprisingly is failing to compile fe_exec.c. > > We have a workaround for that symbol in timezone/private.h: > > #ifndef SIZE_MAX > #define SIZE_MAX ((size_t) -1) > #endif > > and a bit of grepping finds other places that are using the (size_t) -1 > trick explicitly. So what I'm tempted to do is move the above stanza > into c.h. Any objections? Not from me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Missing SIZE_MAX
Commit 2e70d6b5e added a dependency on SIZE_MAX to libpq/fe_exec.c. According to C99 and recent POSIX, that symbol should be provided by , but SUS v2 (POSIX 2001) doesn't require to exist at all ... and I now notice that gaur/pademelon doesn't have it, and unsurprisingly is failing to compile fe_exec.c. We have a workaround for that symbol in timezone/private.h: #ifndef SIZE_MAX #define SIZE_MAX ((size_t) -1) #endif and a bit of grepping finds other places that are using the (size_t) -1 trick explicitly. So what I'm tempted to do is move the above stanza into c.h. Any objections? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers