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

On Thu, Dec 5, 2024 at 10:20 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
> > So apparently, "off_t" was the same as "loff_t" before 962da900a,
> > but it no longer is the same on 32-bit machines.
>
> OK, I see what is happening. On platforms that need it, we define
> _FILE_OFFSET_BITS as 64 in pg_config.h to ensure that off_t is
> big enough. However, 962da900a did this:
>
> --- a/src/include/postgres_ext.h
> +++ b/src/include/postgres_ext.h
> @@ -23,7 +23,7 @@
> #ifndef POSTGRES_EXT_H
> #define POSTGRES_EXT_H
>
> -#include "pg_config_ext.h"
> +#include <stdint.h>
>
> Since c.h reads postgres_ext.h first, <stdint.h> is now pulled in
> before pg_config.h, and that in turn pulls in <features.h>, which
> locks down the decision that off_t will be 32 bits, as well as some
> other decisions we don't want. We can NOT include any system headers
> before pg_config.h; I'm surprised we're not seeing related failures on
> the Solaris-en, where _LARGEFILE_SOURCE is similarly critical.

Ah, right. Oops.

> Another rather serious problem here is that we no longer provide
> macro PG_INT64_TYPE, which seems rather likely to break applications
> that were relying on it. That is part of our external API, we
> can't just remove it on a whim.

I had concluded that PG_INT64_TYPE wasn't part of our external API but
pg_int64 was, based on the comment:

/* Define a signed 64-bit integer type for use in client API declarations. */
-typedef PG_INT64_TYPE pg_int64;
+typedef int64_t pg_int64;

But yeah, I obviously hadn't considered that other interaction, and ...

> I think the least painful solution would be to revert the parts
> of 962da900a that got rid of pg_config_ext.h and PG_INT64_TYPE.
> Since PG_INT64_TYPE is a macro not a typedef, it might be okay
> to #define it as int64_t even before we've read that header,
> so as not to give up the principle of relying on stdint.h for the
> underlying definition.

... yeah that sounds like a plan. Looking into it.

> Now that I see this, I'm fairly astonished that there aren't
> more problems than we've noticed. I wonder whether it'd be
> a good idea to put in a static assert somewhere about the
> width of off_t, so that the next screwup of this sort will
> be easier to diagnose.

Right, we could assert that it hasn't changed from what configure detected.