On Thu, Dec 5, 2024 at 10:58 AM Thomas Munro <thomas.mu...@gmail.com> wrote: > On Thu, Dec 5, 2024 at 10:55 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > Could another way be to read pg_config.h before postgres_ext.h? > > I think the current order was intentional, but maybe the > > disadvantages outweigh the advantages now. > > Yeah I was just testing that idea :-) I can't see why it needs to be > first, but was looking for what the original reason was...
Seems good to me. Also there were another couple of contortions due to the older ordering, which we could improve I think?
From 36e93b7c51dbc8a8d5a9b857a3616edb18c914a6 Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Thu, 5 Dec 2024 11:06:28 +1300 Subject: [PATCH 1/2] Fix header inclusion order in c.h. Commit 962da900a added #include <stdint.h> to postgres_ext.h, which broke c.h's header ordering rule. The system headers on some systems would then lock down off_t's size in other private macros, before they'd had a chance to see our definition of _FILE_OFFSET_BITS (and perhaps did the same for other things). This was picked up by perl's ABI compatibility checks on some 32 bit systems in the build farm. Move #include "postgres_ext.h" below that, because there isn't any current reason for the original ordering. Diagnosed-by: Tom Lane <t...@sss.pgh.pa.us> Discussion: https://postgr.es/m/2397643.1733347237%40sss.pgh.pa.us --- src/include/c.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/include/c.h b/src/include/c.h index 734626e75a8..1271137f5df 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -46,11 +46,10 @@ #ifndef C_H #define C_H -#include "postgres_ext.h" - #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 */ +#include "postgres_ext.h" /* includes a system header file */ /* System header files that should be available everywhere in Postgres */ #include <inttypes.h> -- 2.47.0
From caa89e1176977f26167e0b7ca8b6396a7e4dbba9 Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Thu, 5 Dec 2024 11:31:52 +1300 Subject: [PATCH 2/2] Add required headers to postgres_ext.h. A couple of macros defined by postgres_ext.h historically required their users to include necessary headers themselves before use. They couldn't be included by postgres_ext.h itself due to inclusion ordering rules in c.h. Since we've now changed the order in c.h, we might as well be nicer to users by including them. --- src/include/postgres_ext.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/include/postgres_ext.h b/src/include/postgres_ext.h index 202eb049622..88480402d8a 100644 --- a/src/include/postgres_ext.h +++ b/src/include/postgres_ext.h @@ -23,7 +23,9 @@ #ifndef POSTGRES_EXT_H #define POSTGRES_EXT_H +#include <limits.h> #include <stdint.h> +#include <stdlib.h> /* * Object ID is a fundamental type in Postgres. @@ -37,10 +39,7 @@ typedef unsigned int Oid; #endif #define OID_MAX UINT_MAX -/* you will need to include <limits.h> to use the above #define */ - #define atooid(x) ((Oid) strtoul((x), NULL, 10)) -/* the above needs <stdlib.h> */ /* Define a signed 64-bit integer type for use in client API declarations. */ -- 2.47.0