On Thu, 18 Apr 2024 at 08:31, Thomas Munro <thomas.mu...@gmail.com> wrote:
> On Sat, Mar 23, 2024 at 3:23 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
>> Thomas Munro <thomas.mu...@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 long unsigned 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
