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

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.