Thread: [HACKERS] Missing SIZE_MAX

[HACKERS] Missing SIZE_MAX

From
Tom Lane
Date:
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



Re: [HACKERS] Missing SIZE_MAX

From
Robert Haas
Date:
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



Re: [HACKERS] Missing SIZE_MAX

From
Alvaro Herrera
Date:
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



Re: [HACKERS] Missing SIZE_MAX

From
Tom Lane
Date:
[ 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



Re: [HACKERS] Missing SIZE_MAX

From
Robert Haas
Date:
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



Re: [HACKERS] Missing SIZE_MAX

From
Tom Lane
Date:
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



Re: [HACKERS] Missing SIZE_MAX

From
Tom Lane
Date:
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