Re: Cannot find a working 64-bit integer type on Illumos - Mailing list pgsql-hackers
From | Peter Eisentraut |
---|---|
Subject | Re: Cannot find a working 64-bit integer type on Illumos |
Date | |
Msg-id | b936d2fb-590d-49c3-a615-92c3a88c6c19@eisentraut.org Whole thread Raw |
In response to | Re: Cannot find a working 64-bit integer type on Illumos (Thomas Munro <thomas.munro@gmail.com>) |
Responses |
Re: Cannot find a working 64-bit integer type on Illumos
|
List | pgsql-hackers |
On 04.07.24 03:55, Thomas Munro wrote: > Unfortunately, that theory turned out to be wrong. The usual suspect, > Windows, uses something else: "I64" or something like that. We could > teach our snprintf to grok that, but I don't like the idea anymore. > So let's go back to INT64_MODIFIER, with just a small amount of > configure time work to pick the right value. I couldn't figure out > any header-only way to do that. src/port/snprintf.c used to support I64 in the past, but it was later removed. We could probably put it back. >> Personally, I find "PRId64" pretty unreadable. "INT64_MODIFIER" wasn't >> nice either, though, and following standards is good, so I'm sure I'll >> get used to it. Using PRId64 would be very beneficial because gettext understands it, and so we would no longer need the various workarounds for not putting INT64_FORMAT into the middle of a translated string. But this could be a separate patch. What you have works for now. Here are some other comments on this patch set: * v3-0001-Use-int64_t-support-from-stdint.h.patch - src/include/c.h: Maybe add a comment that above all the int8_t -> int8 etc. typedefs that these are for backward compatibility or something like that. Actually, just move the comment +/* Historical names for limits in stdint.h. */ up a bit to it covers the types as well. Also, these /* == 8 bits */ comments could be removed, I think. - src/include/port/pg_bitutils.h: - src/port/pg_bitutils.c: - src/port/snprintf.c: These changes look functionally correct, but I think I like the old code layout better, like #if (using long) ... #elif (using long long) ... #else #error #endif rather than #if (using long) ... #else static assertion ... // long long #endif which seems a bit more complicated. I think you could leave the code mostly alone and just change defined(HAVE_LONG_INT_64) to SIZEOF_LONG == 8 defined(HAVE_LONG_LONG_INT_64) to SIZEOF_LONG_LONG == 8 in each case. - src/include/postgres_ext.h: -#define OID_MAX UINT_MAX -/* you will need to include <limits.h> to use the above #define */ +#define OID_MAX UINT32_MAX If the type Oid is unsigned int, then the OID_MAX should be UINT_MAX. So this should not be changed. Also, is the comment about <limits.h> no longer applicable? - src/interfaces/ecpg/ecpglib/typename.c: - src/interfaces/ecpg/include/sqltypes.h: - .../test/expected/compat_informix-sqlda.c: -#ifdef HAVE_LONG_LONG_INT_64 +#if SIZEOF_LONG < 8 These changes alter the library behavior unnecessarily. The old code would always prefer to report back long long (ECPGt_long_long etc.), but the new code will report back long (ECPGt_long etc.) if it is 64-bit. I don't know the impact of these changes, but it seems preferable to keep the existing behavior. - src/interfaces/ecpg/include/ecpg_config.h.in: - src/interfaces/ecpg/include/meson.build: In the past, we have kept obsolete symbols as always defined in ecpg_config.h. We ought to do the same here. * v3-0002-Remove-traces-of-BeOS.patch This looks ok. This could also be committed before 0001. * v3-0003-Allow-tzcode-to-use-stdint.h-and-inttypes.h.patch - src/timezone/localtime.c: Addition of #include <stdint.h> is unnecessary, since it's already included in c.h, and it's also not in the upstream code. This looks like a typo: - * UTC months are at least 28 days long (minus 1 second for a + * UTC months are at least 2 days long (minus 1 second for a -getsecs(const char *strp, int32 *const secsp) +getsecs(const char *strp, int_fast32_t * const secsp) Need to add int_fast32_t (and maybe the other types) to typedefs.list? - src/timezone/zic.c: +#include <inttypes.h> +#include <stdint.h> We don't need both of these. Also, this is not in the upstream code AFAICT.
pgsql-hackers by date: