Re: Cannot find a working 64-bit integer type on Illumos

On Thu, 18 Apr 2024 at 08:31, Thomas Munro <thomas.munro@gmail.com> wrote:
> On Sat, Mar 23, 2024 at 3:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Thomas Munro <thomas.munro@gmail.com> writes:
>> > . o O ( int64_t, PRIdi64, etc were standardised a quarter of a century ago )
>>
>> Yeah.  Now that we require C99 it's probably reasonable to assume
>> that those things exist.  I wouldn't be in favor of ripping out our
>> existing notations like UINT64CONST, because the code churn would be
>> substantial and the gain minimal.  But we could imagine reimplementing
>> that stuff atop <stdint.h> and then getting rid of the configure-time
>> probes.
>
> I played around with this a bit, but am not quite there yet.
>
> printf() is a little tricky.  The standard wants us to use
> <inttypes.h>'s PRId64 etc, but that might confuse our snprintf.c (in
> theory, probably not in practice).  "ll" should have the right size on
> all systems, but gets warnings from the printf format string checker
> on systems where "l" is the right type.  So I think we still need to
> probe for INT64_MODIFIER at configure-time.  Here's one way, but I can
> see it's not working on Clang/Linux... perhaps instead of that dubious
> incantation I should try compiling some actual printfs and check for
> warnings/errors.
>
> I think INT64CONST should just point to standard INT64_C().
>
> For limits, why do we have this:
>
> - * stdint.h limits aren't guaranteed to have compatible types with our fixed
> - * width types. So just define our own.
>
> ?  I mean, how could they not have compatible types?
>
> I noticed that configure.ac checks if int64 (no "_t") might be defined
> already by system header pollution, but meson.build doesn't.  That's
> an inconsistency that should be fixed, but which way?  Hmm, commit
> 15abc7788e6 said that was done for BeOS, which we de-supported.  So
> maybe we should get rid of that?

Thanks for working on this!  I test the patch and it now works on Illumos when
configure with -Werror option.  However, there are some errors when compiling.

In file included from /home/japin/postgres/build/../src/include/c.h:834,
                 from /home/japin/postgres/build/../src/include/postgres_fe.h:25,
                 from /home/japin/postgres/build/../src/common/config_info.c:20:
/home/japin/postgres/build/../src/common/config_info.c: In function 'get_configdata':
/home/japin/postgres/build/../src/common/config_info.c:198:11: error: comparison of integer expressions of different
signedness:'int' and 'size_t' {aka 'long unsigned int'} [-Werror=sign-compare] 
  198 |  Assert(i == *configdata_len);
      |           ^~
/home/japin/postgres/build/../src/common/config_info.c:198:2: note: in expansion of macro 'Assert'
  198 |  Assert(i == *configdata_len);
      |  ^~~~~~
In file included from /home/japin/postgres/build/../src/common/blkreftable.c:36:
/home/japin/postgres/build/../src/include/lib/simplehash.h: In function 'blockreftable_stat':
/home/japin/postgres/build/../src/include/lib/simplehash.h:1138:9: error: format '%llu' expects argument of type 'long
longunsigned int', but argument 4 has type 'uint64' {aka 'long unsigned int'} [-Werror=format=] 
 1138 |  sh_log("size: " UINT64_FORMAT ", members: %u, filled: %f, total chain: %u, max chain: %u, avg chain: %f,
total_collisions:%u, max_collisions: %u, avg_collisions: %f", 
      |         ^~~~~~~~
 1139 |      tb->size, tb->members, fillfactor, total_chain_length, max_chain_length, avg_chain_length,
      |      ~~~~~~~~
      |        |
      |        uint64 {aka long unsigned int}
/home/japin/postgres/build/../src/include/common/logging.h:125:46: note: in definition of macro 'pg_log_info'
  125 |  pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__)
      |                                              ^~~~~~~~~~~
/home/japin/postgres/build/../src/include/lib/simplehash.h:1138:2: note: in expansion of macro 'sh_log'
 1138 |  sh_log("size: " UINT64_FORMAT ", members: %u, filled: %f, total chain: %u, max chain: %u, avg chain: %f,
total_collisions:%u, max_collisions: %u, avg_collisions: %f", 
      |  ^~~~~~
In file included from /home/japin/postgres/build/../src/include/access/xlogrecord.h:14,
                 from /home/japin/postgres/build/../src/include/access/xlogreader.h:41,
                 from /home/japin/postgres/build/../src/include/access/xlog_internal.h:23,
                 from /home/japin/postgres/build/../src/common/controldata_utils.c:28:
/home/japin/postgres/build/../src/include/access/rmgr.h: In function 'RmgrIdIsCustom':
/home/japin/postgres/build/../src/include/access/rmgr.h:50:42: error: comparison of integer expressions of different
signedness:'int' and 'unsigned int' [-Werror=sign-compare] 
   50 |  return rmid >= RM_MIN_CUSTOM_ID && rmid <= RM_MAX_CUSTOM_ID;
      |                                          ^~
/home/japin/postgres/build/../src/common/blkreftable.c: In function 'BlockRefTableReaderGetBlocks':
/home/japin/postgres/build/../src/common/blkreftable.c:716:22: error: comparison of integer expressions of different
signedness:'unsigned int' and 'int' [-Werror=sign-compare] 
  716 |         blocks_found < nblocks)
      |                      ^
/home/japin/postgres/build/../src/common/blkreftable.c:732:22: error: comparison of integer expressions of different
signedness:'unsigned int' and 'int' [-Werror=sign-compare] 
  732 |         blocks_found < nblocks)
      |                      ^
/home/japin/postgres/build/../src/common/blkreftable.c:742:20: error: comparison of integer expressions of different
signedness:'unsigned int' and 'int' [-Werror=sign-compare] 
  742 |   if (blocks_found >= nblocks)
      |                    ^~
cc1: all warnings being treated as errors
cc1: all warnings being treated as errors
make[2]: *** [../../src/Makefile.global:947: config_info.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: *** [../../src/Makefile.global:947: controldata_utils.o] Error 1
In file included from /home/japin/postgres/build/../src/include/postgres_fe.h:25,
                 from /home/japin/postgres/build/../src/common/logging.c:15:
/home/japin/postgres/build/../src/common/logging.c: In function 'pg_log_generic_v':
/home/japin/postgres/build/../src/include/c.h:523:23: error: format '%llu' expects argument of type 'long long unsigned
int',but argument 3 has type 'uint64' {aka 'long unsigned int'} [-Werror=format=] 
  523 | #define UINT64_FORMAT "%" INT64_MODIFIER "u"
      |                       ^~~
/home/japin/postgres/build/../src/common/logging.c:259:21: note: in expansion of macro 'UINT64_FORMAT'
  259 |     fprintf(stderr, UINT64_FORMAT ":", lineno);
      |                     ^~~~~~~~~~~~~
/home/japin/postgres/build/../src/include/c.h:523:43: note: format string is defined here
  523 | #define UINT64_FORMAT "%" INT64_MODIFIER "u"
      |                        ~~~~~~~~~~~~~~~~~~~^
      |                                           |
      |                                           long long unsigned int
/home/japin/postgres/build/../src/common/unicode_norm.c: In function 'recompose_code':
/home/japin/postgres/build/../src/common/unicode_norm.c:290:17: error: comparison of integer expressions of different
signedness:'int' and 'long unsigned int' [-Werror=sign-compare] 
  290 |   for (i = 0; i < lengthof(UnicodeDecompMain); i++)
      |                 ^
In file included from /home/japin/postgres/build/../src/include/c.h:834,
                 from /home/japin/postgres/build/../src/common/encnames.c:13:
/home/japin/postgres/build/../src/common/encnames.c: In function 'pg_encoding_to_char_private':
/home/japin/postgres/build/../src/common/encnames.c:593:19: error: comparison of integer expressions of different
signedness:'int' and 'pg_enc' [-Werror=sign-compare] 
  593 |   Assert(encoding == p->encoding);
      |                   ^~
/home/japin/postgres/build/../src/common/encnames.c:593:3: note: in expansion of macro 'Assert'
  593 |   Assert(encoding == p->encoding);
      |   ^~~~~~
/home/japin/postgres/build/../src/common/jsonapi.c: In function 'pg_parse_json_incremental':
/home/japin/postgres/build/../src/common/jsonapi.c:693:11: error: comparison of integer expressions of different
signedness:'char' and 'JsonTokenType' [-Werror=sign-compare] 
  693 |   if (top == tok)
      |           ^~
cc1: all warnings being treated as errors
cc1: all warnings being treated as errors
make[2]: *** [../../src/Makefile.global:947: logging.o] Error 1
make[2]: *** [../../src/Makefile.global:947: blkreftable.o] Error 1
/home/japin/postgres/build/../src/common/kwlookup.c: In function 'ScanKeywordLookup':
/home/japin/postgres/build/../src/common/kwlookup.c:50:10: error: comparison of integer expressions of different
signedness:'size_t' {aka 'long unsigned int'} and 'int' [-Werror=sign-compare] 
   50 |  if (len > keywords->max_kw_len)
      |          ^
cc1: all warnings being treated as errors
make[2]: *** [../../src/Makefile.global:947: encnames.o] Error 1
cc1: all warnings being treated as errors
cc1: all warnings being treated as errors
make[2]: *** [../../src/Makefile.global:947: kwlookup.o] Error 1
In file included from /home/japin/postgres/build/../src/include/c.h:834,
                 from /home/japin/postgres/build/../src/include/postgres_fe.h:25,
                 from /home/japin/postgres/build/../src/common/file_utils.c:19:
/home/japin/postgres/build/../src/common/file_utils.c: In function 'pg_pwrite_zeros':
/home/japin/postgres/build/../src/common/file_utils.c:725:23: error: comparison of integer expressions of different
signedness:'ssize_t' {aka 'long int'} and 'size_t' {aka 'long unsigned int'} [-Werror=sign-compare] 
  725 |  Assert(total_written == size);
      |                       ^~
/home/japin/postgres/build/../src/common/file_utils.c:725:2: note: in expansion of macro 'Assert'
  725 |  Assert(total_written == size);
      |  ^~~~~~
make[2]: *** [../../src/Makefile.global:947: unicode_norm.o] Error 1
cc1: all warnings being treated as errors
make[2]: *** [../../src/Makefile.global:947: file_utils.o] Error 1
/home/japin/postgres/build/../src/common/unicode_case.c: In function 'convert_case':
/home/japin/postgres/build/../src/common/unicode_case.c:155:31: error: comparison of integer expressions of different
signedness:'size_t' {aka 'long unsigned int'} and 'ssize_t' {aka 'long int'} [-Werror=sign-compare] 
  155 |  while ((srclen < 0 || srcoff < srclen) && src[srcoff] != '\0')
      |                               ^
/home/japin/postgres/build/../src/common/wchar.c: In function 'pg_utf8_verifystr':
/home/japin/postgres/build/../src/common/wchar.c:1868:10: error: comparison of integer expressions of different
signedness:'int' and 'long unsigned int' [-Werror=sign-compare] 
 1868 |  if (len >= STRIDE_LENGTH)
      |          ^~
/home/japin/postgres/build/../src/common/wchar.c:1870:14: error: comparison of integer expressions of different
signedness:'int' and 'long unsigned int' [-Werror=sign-compare] 
 1870 |   while (len >= STRIDE_LENGTH)
      |              ^~
cc1: all warnings being treated as errors
make[2]: *** [../../src/Makefile.global:947: unicode_case.o] Error 1
cc1: all warnings being treated as errors
make[2]: *** [../../src/Makefile.global:947: jsonapi.o] Error 1
cc1: all warnings being treated as errors
make[2]: *** [../../src/Makefile.global:947: wchar.o] Error 1
make[1]: *** [Makefile:42: all-common-recurse] Error 2
make: *** [GNUmakefile:11: all-src-recurse] Error 2

For rmid >= RM_MIN_CUSTOM_ID && rmid <= RM_MAX_CUSTOM_ID comparison error, I
found that UINT8_MAX is defined as '255U' on Illumos, however, Linux glibc
uses '255' for UINT8_MAX, which is signed.

[1] https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/sys/int_limits.h#L92
[2]
https://sourceware.org/git/?p=glibc.git;a=blob;f=stdlib/stdint.h;h=bb3e8b5cc61fb3df8842225d2286de67e6f2ffe2;hb=refs/heads/master#l116


--
Regards,
Japin Li