From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Tomas Vondra <tomas(at)vondra(dot)me>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Japin Li <japinli(at)hotmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Cannot find a working 64-bit integer type on Illumos |
Date: | 2024-12-05 00:21:46 |
Message-ID: | CA+hUKGK0b-7Mzj6hAvwoB86QaYnoHGQ4qPtJNC5oqqJpWVVwtA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Dec 5, 2024 at 12:16 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> In c.h, I'd put in a very explicit comment in front of pg_config.h.
> Also, I don't like making it look like postgres_ext.h is of the
> same ilk as the config headers, since it isn't. So maybe like
>
> /*
> * These headers must be included before any system headers, because
> * on some platforms they affect the behavior of the system headers
> * (for example, by defining _FILE_OFFSET_BITS).
> */
> #include "pg_config.h"
> #include "pg_config_manual.h" /* must be after pg_config.h */
> #include "pg_config_os.h" /* must be before any system header files */
>
> /* Pull in fundamental symbols that we also expose to applications */
> #include "postgres_ext.h"
>
> /* System header files that should be available everywhere in Postgres */
> #include <inttypes.h>
> ...
>
> The comment for pg_config_os.h is redundant this way, maybe we could
> rewrite it as something more useful?
OK, how about:
+#include "pg_config_os.h" /* config from
include/port/PORTNAME.h */
That demystifies where it's really coming from ("portname" is used in
the meson script, and seems more informative than "template").
> Also, there's probably no reason anymore that postgres_ext.h couldn't
> be placed after those fundamental system headers, and that might be
> clearer. (I think perhaps the main reason for the existing ordering
> was to demonstrate that postgres_ext.h could be included before any
> system headers, and that's no longer a consideration.)
Yeah, that makes sense.
> I don't especially care for your proposed changes to postgres_ext.h.
> That substantially expands the footprint of what gets defined by
> pulling that in, and some users might not care for that. (For
> example, because they have ordering assumptions similar to what we're
> dealing with here.) Now you already snuck the camel's nose under the
> tent by including stdint.h there, and maybe these additional headers
> wouldn't do any further damage. But I don't see a strong argument to
> change long-standing external APIs any more than we absolutely must.
If someone wants to define things like that before potentially
including system headers, they're not going about it the right way if
they're including our headers first (or anything at all not under
their direct control). But OK, I can work with the
not-broken-so-don't-fix-it and
pulling-in-more-stuff-that-maybe-they-don't-want arguments. :-)
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Fix-header-inclusion-order-in-c.h.patch | text/x-patch | 1.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Smith | 2024-12-05 00:35:20 | Re: Introduce XID age and inactive timeout based replication slot invalidation |
Previous Message | Jeff Davis | 2024-12-05 00:21:34 | Re: Collation & ctype method table, and extension hooks |