Thread: [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 <stdint.h>, but SUS v2 (POSIX 2001) doesn't require <stdint.h>
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
On Fri, Sep 1, 2017 at 11:09 AM, Tom Lane <tgl@sss.pgh.pa.us> 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 <stdint.h>, but SUS v2 (POSIX 2001) doesn't require <stdint.h> > 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
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 Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
[ warning: more than you really wanted to know ahead ]
Alvaro Herrera <alvherre@alvh.no-ip.org> 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
On Fri, Sep 1, 2017 at 12:22 PM, Tom Lane <tgl@sss.pgh.pa.us> 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
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Sep 1, 2017 at 12:22 PM, Tom Lane <tgl@sss.pgh.pa.us> 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
I wrote:
> Robert Haas <robertmhaas@gmail.com> 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 (0xFFFF)
#define PG_INT32_MIN (-0x7FFFFFFF-1)
#define PG_INT32_MAX (0x7FFFFFFF)
! #define PG_UINT32_MAX (0xFFFFFFFF)
#define PG_INT64_MIN (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
#define PG_INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#define PG_UINT64_MAX UINT64CONST(0xFFFFFFFFFFFFFFFF)
/* 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 (0xFFFF)
#define PG_INT32_MIN (-0x7FFFFFFF-1)
#define PG_INT32_MAX (0x7FFFFFFF)
! #define PG_UINT32_MAX (0xFFFFFFFFU)
#define PG_INT64_MIN (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
#define PG_INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#define PG_UINT64_MAX UINT64CONST(0xFFFFFFFFFFFFFFFF)
/* 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