Re: Missing SIZE_MAX

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: pgsql-hackers(at)postgreSQL(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: Missing SIZE_MAX
Date: 2017-09-01 16:22:21
Message-ID: 18781.1504282941@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

[ warning: more than you really wanted to know ahead ]

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2017-09-01 16:24:56 Re: [PATCH] Assert that the correct locks are held when calling PageGetLSN()
Previous Message Magnus Hagander 2017-09-01 16:22:20 Re: log_destination=file