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.
