Re: RELEASE STOPPER? nonportable int64 constants in pg_crc.c

Recent changes in pg_crc.c (64 bit CRC) introduced non portable constants of the form:

-c -o pg_crc.o pg_crc.c
287 | 0x0000000000000000, 0x42F0E1EBA9EA3693,
............................a..................
a - 1506-207 (W) Integer constant 0x42F0E1EBA9EA3693 out of range.

I guess this will show up on a lot of non gcc platforms !!!!!
It shows no diffs in the regression tests! From what I understand,
failure would only show up after fast shutdown/crash.

Attached is a patch, but I have no idea how portable that is.

Andreas

Zeugswetter Andreas SB writes:

>
> Recent changes in pg_crc.c (64 bit CRC) introduced non portable constants of the form:
>
> -c -o pg_crc.o pg_crc.c
> 287 | 0x0000000000000000, 0x42F0E1EBA9EA3693,
> ............................a..................
> a - 1506-207 (W) Integer constant 0x42F0E1EBA9EA3693 out of range.
>
> I guess this will show up on a lot of non gcc platforms !!!!!
> It shows no diffs in the regression tests! From what I understand,
> failure would only show up after fast shutdown/crash.
>
> Attached is a patch, but I have no idea how portable that is.

I don't think it's the answer either. The patch assumes that int64 ==
long long. The ugly solution might have to be:

#if <int64 == long>
#define L64 L
#else
#define L64 LL
#endif

const uint64 crc_table[256] = {
0x0000000000000000##L64, 0x42F0E1EBA9EA3693##L64,
0x85E1C3D753D46D26##L64, 0xC711223CFA3E5BB5##L64,
...

--
Peter Eisentraut peter_e(at)gmx(dot)net http://yi.org/peter-e/

Zeugswetter Andreas SB <ZeugswetterA(at)wien(dot)spardat(dot)at> writes:
> Recent changes in pg_crc.c (64 bit CRC) introduced non portable constants of the form:

> -c -o pg_crc.o pg_crc.c
> 287 | 0x0000000000000000, 0x42F0E1EBA9EA3693,
> ............................a..................
> a - 1506-207 (W) Integer constant 0x42F0E1EBA9EA3693 out of range.

Please observe that this is a warning, not an error. Your proposed
fix is considerably worse than the disease, because it will break on
compilers that do not recognize "LL" constants, to say nothing of
machines where L is correct and LL is some yet wider datatype.

I'm aware that some compilers will produce warnings about these
constants, but there should not be any that fail completely, since
(a) we won't be compiling this code unless we've proven that the
compiler supports a 64-bit-int datatype, and (b) the C standard
forbids a compiler from requiring width suffixes (cf. 6.4.4.1 in C99).

I don't think it's a good tradeoff to risk breaking some platforms in
order to suppress warnings from overly anal-retentive compilers.

regards, tom lane

okay, this was the only one I was waiting to hear on ... the fix committed
this afternoon for the regression test, did/does it fix the problem? are
we safe on a proper RC1 now?

On Wed, 21 Mar 2001, Tom Lane wrote:

> Zeugswetter Andreas SB <ZeugswetterA(at)wien(dot)spardat(dot)at> writes:
> > Recent changes in pg_crc.c (64 bit CRC) introduced non portable constants of the form:
>
> > -c -o pg_crc.o pg_crc.c
> > 287 | 0x0000000000000000, 0x42F0E1EBA9EA3693,
> > ............................a..................
> > a - 1506-207 (W) Integer constant 0x42F0E1EBA9EA3693 out of range.
>
> Please observe that this is a warning, not an error. Your proposed
> fix is considerably worse than the disease, because it will break on
> compilers that do not recognize "LL" constants, to say nothing of
> machines where L is correct and LL is some yet wider datatype.
>
> I'm aware that some compilers will produce warnings about these
> constants, but there should not be any that fail completely, since
> (a) we won't be compiling this code unless we've proven that the
> compiler supports a 64-bit-int datatype, and (b) the C standard
> forbids a compiler from requiring width suffixes (cf. 6.4.4.1 in C99).
>
> I don't think it's a good tradeoff to risk breaking some platforms in
> order to suppress warnings from overly anal-retentive compilers.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>

Marc G. Fournier ICQ#7615664 IRC Nick: Scrappy
Systems Administrator @ hub.org
primary: scrappy(at)hub(dot)org secondary: scrappy(at){freebsd|postgresql}.org

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> I don't think it's the answer either. The patch assumes that int64 ==
> long long. The ugly solution might have to be:

> #if <int64 == long>
> #define L64 L
> #else
> #define L64 LL
> #endif

> const uint64 crc_table[256] = {
> 0x0000000000000000##L64, 0x42F0E1EBA9EA3693##L64,
> 0x85E1C3D753D46D26##L64, 0xC711223CFA3E5BB5##L64,

Hmm ... how portable is that likely to be? I don't want to suppress
warnings on a few boxes at the cost of breaking even one platform
that would otherwise work. See my reply to Andreas.

regards, tom lane

> Zeugswetter Andreas SB <ZeugswetterA(at)wien(dot)spardat(dot)at> writes:
> > Recent changes in pg_crc.c (64 bit CRC) introduced non portable constants of the form:
>
> > -c -o pg_crc.o pg_crc.c
> > 287 | 0x0000000000000000, 0x42F0E1EBA9EA3693,
> > ............................a..................
> > a - 1506-207 (W) Integer constant 0x42F0E1EBA9EA3693 out of range.
>
> Please observe that this is a warning, not an error. Your proposed
> fix is considerably worse than the disease, because it will break on
> compilers that do not recognize "LL" constants, to say nothing of
> machines where L is correct and LL is some yet wider datatype.

I am seeing the same warnings with gcc 2.7.2.1 -Wall on BSDi i386:

pg_crc.c:353: warning: integer constant out of range
pg_crc.c:353: warning: integer constant out of range
pg_crc.c:354: warning: integer constant out of range
pg_crc.c:354: warning: integer constant out of range
pg_crc.c:355: warning: integer constant out of range
pg_crc.c:355: warning: integer constant out of range
pg_crc.c:356: warning: integer constant out of range

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Can we use (long long) rather than LL?

> > Zeugswetter Andreas SB <ZeugswetterA(at)wien(dot)spardat(dot)at> writes:
> > > Recent changes in pg_crc.c (64 bit CRC) introduced non portable constants of the form:
> >
> > > -c -o pg_crc.o pg_crc.c
> > > 287 | 0x0000000000000000, 0x42F0E1EBA9EA3693,
> > > ............................a..................
> > > a - 1506-207 (W) Integer constant 0x42F0E1EBA9EA3693 out of range.
> >
> > Please observe that this is a warning, not an error. Your proposed
> > fix is considerably worse than the disease, because it will break on
> > compilers that do not recognize "LL" constants, to say nothing of
> > machines where L is correct and LL is some yet wider datatype.
>
> I am seeing the same warnings with gcc 2.7.2.1 -Wall on BSDi i386:
>
> pg_crc.c:353: warning: integer constant out of range
> pg_crc.c:353: warning: integer constant out of range
> pg_crc.c:354: warning: integer constant out of range
> pg_crc.c:354: warning: integer constant out of range
> pg_crc.c:355: warning: integer constant out of range
> pg_crc.c:355: warning: integer constant out of range
> pg_crc.c:356: warning: integer constant out of range
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
> + If your life is a hard drive, | 830 Blythe Avenue
> + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Can we use (long long) rather than LL?

No.

regards, tom lane

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Can we use (long long) rather than LL?
>
> No.

Can I ask how 0LL is different from (long long)0?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Can we use (long long) rather than LL?
>>
>> No.

> Can I ask how 0LL is different from (long long)0?

The former is a long-long-int constant ab initio. The latter is an int
constant that is subsequently casted to long long. If you write
(long long) 12345678901234567890
I'd expect a compiler that warns about larger-than-int constants to
produce a warning anyway, since the warning is only looking at the
constant and not its context of use. (If the warning had that much
intelligence, it'd not be complaining now.)

regards, tom lane

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> const uint64 crc_table[256] = {
> 0x0000000000000000##L64, 0x42F0E1EBA9EA3693##L64,
> 0x85E1C3D753D46D26##L64, 0xC711223CFA3E5BB5##L64,
>>
>> Hmm ... how portable is that likely to be?

> If the 'L' or 'LL' suffix is portable (probably), and token pasting is
> portable (yes), then the aggregate should be as well, because one is
> handled by the preprocessor and the other by the compiler.

It's just that I've never seen token-pasting applied to build anything
but identifiers and strings. In theory it should work, but theory does
not always predict what compiler writers will choose to warn about and
where. That "oversized integer" warning could be coming out of the
preprocessor.

BTW, my C book only talks about token-pasting as a step of macro body
expansion. Wouldn't we really need something like

SIXTYFOUR(0x0000000000000000), SIXTYFOUR(0x42F0E1EBA9EA3693),
SIXTYFOUR(0x85E1C3D753D46D26), SIXTYFOUR(0xC711223CFA3E5BB5),

where SIXTYFOUR(x) is conditionally defined to be "x##LL", "x##L",
or perhaps just "x"?

regards, tom lane

Tom Lane writes:

> > #if <int64 == long>
> > #define L64 L
> > #else
> > #define L64 LL
> > #endif
>
> > const uint64 crc_table[256] = {
> > 0x0000000000000000##L64, 0x42F0E1EBA9EA3693##L64,
> > 0x85E1C3D753D46D26##L64, 0xC711223CFA3E5BB5##L64,
>
> Hmm ... how portable is that likely to be?

If the 'L' or 'LL' suffix is portable (probably), and token pasting is
portable (yes), then the aggregate should be as well, because one is
handled by the preprocessor and the other by the compiler.

> I don't want to suppress warnings on a few boxes at the cost of
> breaking even one platform that would otherwise work. See my reply to
> Andreas.

It's possible that there might be one that warns and truncates, but that's
unlikely. Why are there suffixes for integer (not float) constants
anyway?

--
Peter Eisentraut peter_e(at)gmx(dot)net http://yi.org/peter-e/