Add missing PGDLLIMPORT markings

Started by Peter Eisentraut9 months ago10 messages
#1Peter Eisentraut
peter@eisentraut.org
1 attachment(s)

I ran src/tools/mark_pgdllimport.pl and it detected a few new global
variables with missing markings. See attached patch. Please point out
if any of these should not be marked or if they are special cases in
some other way. I'm Cc'ing various people involved with that new code.

Btw., this new variable in memutils.h

extern dsa_area *area;

could probably do with a less generic name?

Attachments:

0001-WIP-Add-missing-PGDLLIMPORT-markings.patchtext/plain; charset=UTF-8; name=0001-WIP-Add-missing-PGDLLIMPORT-markings.patchDownload
From e9d868a38b50c1fa449258926c40cef88bd73045 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 9 Apr 2025 11:55:48 +0200
Subject: [PATCH] WIP: Add missing PGDLLIMPORT markings

Cc: Daniel Gustafsson <daniel@yesql.se> (commit b3f0be788af, 042a66291b0)
Cc: Jacob Champion <jacob.champion@enterprisedb.com> (commit b3f0be788af)
Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi> (commit a78af042701)
Cc: Andres Freund <andres@anarazel.de> (commit 047cba7fa0f, 50cb7505b301)
Cc: Rahila Syed <rahilasyed90@gmail.com> (commit 042a66291b0)
---
 src/include/libpq/oauth.h           | 2 +-
 src/include/postmaster/postmaster.h | 4 ++--
 src/include/storage/bufmgr.h        | 4 ++--
 src/include/storage/md.h            | 2 +-
 src/include/storage/smgr.h          | 2 +-
 src/include/utils/memutils.h        | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h
index 2c6892ffba4..3f4c9acd8b0 100644
--- a/src/include/libpq/oauth.h
+++ b/src/include/libpq/oauth.h
@@ -91,7 +91,7 @@ typedef const OAuthValidatorCallbacks *(*OAuthValidatorModuleInit) (void);
 extern PGDLLEXPORT const OAuthValidatorCallbacks *_PG_oauth_validator_module_init(void);
 
 /* Implementation */
-extern const pg_be_sasl_mech pg_be_oauth_mech;
+extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech;
 
 /*
  * Ensure a validator named in the HBA is permitted by the configuration.
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 39566ee2bd5..92497cd6a0f 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -48,7 +48,7 @@ typedef struct
 } PMChild;
 
 #ifdef EXEC_BACKEND
-extern int	num_pmchild_slots;
+extern PGDLLIMPORT int num_pmchild_slots;
 #endif
 
 /* GUC options */
@@ -117,7 +117,7 @@ pg_noreturn extern void SubPostmasterMain(int argc, char *argv[]);
 #endif
 
 /* defined in pmchild.c */
-extern dlist_head ActiveChildList;
+extern PGDLLIMPORT dlist_head ActiveChildList;
 
 extern void InitPostmasterChildSlots(void);
 extern PMChild *AssignPostmasterChildSlot(BackendType btype);
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 96150a6cfe9..33a8b8c06fb 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -173,8 +173,8 @@ extern PGDLLIMPORT int checkpoint_flush_after;
 extern PGDLLIMPORT int backend_flush_after;
 extern PGDLLIMPORT int bgwriter_flush_after;
 
-extern const PgAioHandleCallbacks aio_shared_buffer_readv_cb;
-extern const PgAioHandleCallbacks aio_local_buffer_readv_cb;
+extern PGDLLIMPORT const PgAioHandleCallbacks aio_shared_buffer_readv_cb;
+extern PGDLLIMPORT const PgAioHandleCallbacks aio_local_buffer_readv_cb;
 
 /* in buf_init.c */
 extern PGDLLIMPORT char *BufferBlocks;
diff --git a/src/include/storage/md.h b/src/include/storage/md.h
index 9d7131eff43..f630b75446c 100644
--- a/src/include/storage/md.h
+++ b/src/include/storage/md.h
@@ -20,7 +20,7 @@
 #include "storage/smgr.h"
 #include "storage/sync.h"
 
-extern const PgAioHandleCallbacks aio_md_readv_cb;
+extern PGDLLIMPORT const PgAioHandleCallbacks aio_md_readv_cb;
 
 /* md storage manager functionality */
 extern void mdinit(void);
diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index 856ebcda350..3964d9334b3 100644
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -74,7 +74,7 @@ typedef SMgrRelationData *SMgrRelation;
 #define SmgrIsTemp(smgr) \
 	RelFileLocatorBackendIsTemp((smgr)->smgr_rlocator)
 
-extern const PgAioTargetInfo aio_smgr_target_info;
+extern PGDLLIMPORT const PgAioTargetInfo aio_smgr_target_info;
 
 extern void smgrinit(void);
 extern SMgrRelation smgropen(RelFileLocator rlocator, ProcNumber backend);
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index d328270fafc..a31b2c699a5 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -400,5 +400,5 @@ extern void HandleGetMemoryContextInterrupt(void);
 extern Size MemoryContextReportingShmemSize(void);
 extern void MemoryContextReportingShmemInit(void);
 extern void AtProcExit_memstats_cleanup(int code, Datum arg);
-extern dsa_area *area;
+extern PGDLLIMPORT dsa_area *area;
 #endif							/* MEMUTILS_H */
-- 
2.49.0

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Eisentraut (#1)
1 attachment(s)
Re: Add missing PGDLLIMPORT markings

On 9 Apr 2025, at 12:02, Peter Eisentraut <peter@eisentraut.org> wrote:

-extern const pg_be_sasl_mech pg_be_oauth_mech;
+extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech;
+1 on this.

Btw., this new variable in memutils.h

extern dsa_area *area;

could probably do with a less generic name?

Good point, unless objected to I'll apply the attached renaming which then also
contain the PGDLLIMPORT addition.

--
Daniel Gustafsson

Attachments:

0001-Rename-global-variable-backing-DSA-area.patchapplication/octet-stream; name=0001-Rename-global-variable-backing-DSA-area.patch; x-unix-mode=0644Download
From 869b01a369a1afa64abf5eb9e9c05f457a4d799c Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Wed, 9 Apr 2025 13:37:30 +0200
Subject: [PATCH] Rename global variable backing DSA area

The global variable backing the DSA area for Memory Context stats
reporting had a too generic name, rename to be more descriptive.

Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/25095db5-b595-4b85-9100-d358907c25b5@eisentraut.org
---
 src/backend/utils/adt/mcxtfuncs.c | 14 +++++------
 src/backend/utils/mmgr/mcxt.c     | 40 +++++++++++++++----------------
 src/include/utils/memutils.h      |  2 +-
 3 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 3ede88e5036..254cdd34fba 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -533,21 +533,21 @@ pg_get_process_memory_contexts(PG_FUNCTION_ARGS)
 	 */
 	Assert(memCxtArea->memstats_dsa_handle != DSA_HANDLE_INVALID);
 	/* Attach to the dsa area if we have not already done so */
-	if (area == NULL)
+	if (MemoryStatsDsaArea == NULL)
 	{
 		MemoryContext oldcontext = CurrentMemoryContext;
 
 		MemoryContextSwitchTo(TopMemoryContext);
-		area = dsa_attach(memCxtArea->memstats_dsa_handle);
+		MemoryStatsDsaArea = dsa_attach(memCxtArea->memstats_dsa_handle);
 		MemoryContextSwitchTo(oldcontext);
-		dsa_pin_mapping(area);
+		dsa_pin_mapping(MemoryStatsDsaArea);
 	}
 
 	/*
 	 * Backend has finished publishing the stats, project them.
 	 */
 	memcxt_info = (MemoryStatsEntry *)
-		dsa_get_address(area, memCxtState[procNumber].memstats_dsa_pointer);
+		dsa_get_address(MemoryStatsDsaArea, memCxtState[procNumber].memstats_dsa_pointer);
 
 #define PG_GET_PROCESS_MEMORY_CONTEXTS_COLS	12
 	for (int i = 0; i < memCxtState[procNumber].total_stats; i++)
@@ -566,7 +566,7 @@ pg_get_process_memory_contexts(PG_FUNCTION_ARGS)
 
 		if (DsaPointerIsValid(memcxt_info[i].name))
 		{
-			name = (char *) dsa_get_address(area, memcxt_info[i].name);
+			name = (char *) dsa_get_address(MemoryStatsDsaArea, memcxt_info[i].name);
 			values[0] = CStringGetTextDatum(name);
 		}
 		else
@@ -574,7 +574,7 @@ pg_get_process_memory_contexts(PG_FUNCTION_ARGS)
 
 		if (DsaPointerIsValid(memcxt_info[i].ident))
 		{
-			ident = (char *) dsa_get_address(area, memcxt_info[i].ident);
+			ident = (char *) dsa_get_address(MemoryStatsDsaArea, memcxt_info[i].ident);
 			values[1] = CStringGetTextDatum(ident);
 		}
 		else
@@ -586,7 +586,7 @@ pg_get_process_memory_contexts(PG_FUNCTION_ARGS)
 		path_datum = (Datum *) palloc(path_length * sizeof(Datum));
 		if (DsaPointerIsValid(memcxt_info[i].path))
 		{
-			path_int = (int *) dsa_get_address(area, memcxt_info[i].path);
+			path_int = (int *) dsa_get_address(MemoryStatsDsaArea, memcxt_info[i].path);
 			for (int j = 0; j < path_length; j++)
 				path_datum[j] = Int32GetDatum(path_int[j]);
 			path_array = construct_array_builtin(path_datum, path_length, INT4OID);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index cf4e22bf1cc..677ee42596f 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -172,7 +172,7 @@ MemoryContext CurTransactionContext = NULL;
 
 /* This is a transient link to the active portal's memory context: */
 MemoryContext PortalContext = NULL;
-dsa_area   *area = NULL;
+dsa_area   *MemoryStatsDsaArea = NULL;
 
 static void MemoryContextDeleteOnly(MemoryContext context);
 static void MemoryContextCallResetCallbacks(MemoryContext context);
@@ -1499,19 +1499,19 @@ ProcessGetMemoryContextInterrupt(void)
 
 		MemoryContextSwitchTo(TopMemoryContext);
 
-		area = dsa_create(memCxtArea->lw_lock.tranche);
+		MemoryStatsDsaArea = dsa_create(memCxtArea->lw_lock.tranche);
 
-		handle = dsa_get_handle(area);
+		handle = dsa_get_handle(MemoryStatsDsaArea);
 		MemoryContextSwitchTo(oldcontext);
 
-		dsa_pin_mapping(area);
+		dsa_pin_mapping(MemoryStatsDsaArea);
 
 		/*
 		 * Pin the DSA area, this is to make sure the area remains attachable
 		 * even if current backend exits. This is done so that the statistics
 		 * are published even if the process exits while a client is waiting.
 		 */
-		dsa_pin(area);
+		dsa_pin(MemoryStatsDsaArea);
 
 		/* Set the handle in shared memory */
 		memCxtArea->memstats_dsa_handle = handle;
@@ -1521,14 +1521,14 @@ ProcessGetMemoryContextInterrupt(void)
 	 * If DSA exists, created by another process publishing statistics, attach
 	 * to it.
 	 */
-	else if (area == NULL)
+	else if (MemoryStatsDsaArea == NULL)
 	{
 		MemoryContext oldcontext = CurrentMemoryContext;
 
 		MemoryContextSwitchTo(TopMemoryContext);
-		area = dsa_attach(memCxtArea->memstats_dsa_handle);
+		MemoryStatsDsaArea = dsa_attach(memCxtArea->memstats_dsa_handle);
 		MemoryContextSwitchTo(oldcontext);
-		dsa_pin_mapping(area);
+		dsa_pin_mapping(MemoryStatsDsaArea);
 	}
 	LWLockRelease(&memCxtArea->lw_lock);
 
@@ -1545,7 +1545,7 @@ ProcessGetMemoryContextInterrupt(void)
 		 * Free any previous allocations, free the name, ident and path
 		 * pointers before freeing the pointer that contains them.
 		 */
-		free_memorycontextstate_dsa(area, memCxtState[idx].total_stats,
+		free_memorycontextstate_dsa(MemoryStatsDsaArea, memCxtState[idx].total_stats,
 									memCxtState[idx].memstats_dsa_pointer);
 	}
 
@@ -1556,10 +1556,10 @@ ProcessGetMemoryContextInterrupt(void)
 	 */
 	memCxtState[idx].total_stats = stats_num;
 	memCxtState[idx].memstats_dsa_pointer =
-		dsa_allocate0(area, stats_num * sizeof(MemoryStatsEntry));
+		dsa_allocate0(MemoryStatsDsaArea, stats_num * sizeof(MemoryStatsEntry));
 
 	meminfo = (MemoryStatsEntry *)
-		dsa_get_address(area, memCxtState[idx].memstats_dsa_pointer);
+		dsa_get_address(MemoryStatsDsaArea, memCxtState[idx].memstats_dsa_pointer);
 
 	if (summary)
 	{
@@ -1572,7 +1572,7 @@ ProcessGetMemoryContextInterrupt(void)
 											 &stat, true);
 		path = lcons_int(1, path);
 		PublishMemoryContext(meminfo, cxt_id, TopMemoryContext, path, stat,
-							 1, area, 100);
+							 1, MemoryStatsDsaArea, 100);
 		cxt_id = cxt_id + 1;
 
 		/*
@@ -1602,7 +1602,7 @@ ProcessGetMemoryContextInterrupt(void)
 			 */
 			memCxtState[idx].total_stats = cxt_id++;
 			PublishMemoryContext(meminfo, cxt_id, c, path,
-								 grand_totals, num_contexts, area, 100);
+								 grand_totals, num_contexts, MemoryStatsDsaArea, 100);
 		}
 		memCxtState[idx].total_stats = cxt_id;
 
@@ -1632,7 +1632,7 @@ ProcessGetMemoryContextInterrupt(void)
 		if (context_id < (max_stats - 1) || stats_count <= max_stats)
 		{
 			/* Copy statistics to DSA memory */
-			PublishMemoryContext(meminfo, context_id, cur, path, stat, 1, area, 100);
+			PublishMemoryContext(meminfo, context_id, cur, path, stat, 1, MemoryStatsDsaArea, 100);
 		}
 		else
 		{
@@ -1657,8 +1657,8 @@ ProcessGetMemoryContextInterrupt(void)
 			int			namelen = strlen("Remaining Totals");
 
 			num_individual_stats = context_id + 1;
-			meminfo[max_stats - 1].name = dsa_allocate(area, namelen + 1);
-			nameptr = dsa_get_address(area, meminfo[max_stats - 1].name);
+			meminfo[max_stats - 1].name = dsa_allocate(MemoryStatsDsaArea, namelen + 1);
+			nameptr = dsa_get_address(MemoryStatsDsaArea, meminfo[max_stats - 1].name);
 			strncpy(nameptr, "Remaining Totals", namelen);
 			meminfo[max_stats - 1].ident = InvalidDsaPointer;
 			meminfo[max_stats - 1].path = InvalidDsaPointer;
@@ -1921,18 +1921,18 @@ AtProcExit_memstats_cleanup(int code, Datum arg)
 	}
 
 	/* If the dsa mapping could not be found, attach to the area */
-	if (area == NULL)
-		area = dsa_attach(memCxtArea->memstats_dsa_handle);
+	if (MemoryStatsDsaArea == NULL)
+		MemoryStatsDsaArea = dsa_attach(memCxtArea->memstats_dsa_handle);
 
 	/*
 	 * Free the memory context statistics, free the name, ident and path
 	 * pointers before freeing the pointer that contains these pointers and
 	 * integer statistics.
 	 */
-	free_memorycontextstate_dsa(area, memCxtState[idx].total_stats,
+	free_memorycontextstate_dsa(MemoryStatsDsaArea, memCxtState[idx].total_stats,
 								memCxtState[idx].memstats_dsa_pointer);
 
-	dsa_detach(area);
+	dsa_detach(MemoryStatsDsaArea);
 	LWLockRelease(&memCxtState[idx].lw_lock);
 }
 
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index d328270fafc..c0987dca155 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -394,11 +394,11 @@ typedef struct MemoryStatsContextId
 
 extern PGDLLIMPORT MemoryStatsBackendState *memCxtState;
 extern PGDLLIMPORT MemoryStatsCtl *memCxtArea;
+extern PGDLLIMPORT dsa_area *MemoryStatsDsaArea;
 extern void ProcessGetMemoryContextInterrupt(void);
 extern const char *ContextTypeToString(NodeTag type);
 extern void HandleGetMemoryContextInterrupt(void);
 extern Size MemoryContextReportingShmemSize(void);
 extern void MemoryContextReportingShmemInit(void);
 extern void AtProcExit_memstats_cleanup(int code, Datum arg);
-extern dsa_area *area;
 #endif							/* MEMUTILS_H */
-- 
2.39.3 (Apple Git-146)

#3Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Daniel Gustafsson (#2)
Re: Add missing PGDLLIMPORT markings

On Wed, Apr 9, 2025 at 4:48 AM Daniel Gustafsson <daniel@yesql.se> wrote:

-extern const pg_be_sasl_mech pg_be_oauth_mech;
+extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech;
+1 on this.

LGTM, too.

Thanks!
--Jacob

#4Andres Freund
andres@anarazel.de
In reply to: Peter Eisentraut (#1)
Re: Add missing PGDLLIMPORT markings

Hi,

On 2025-04-09 12:02:52 +0200, Peter Eisentraut wrote:

I ran src/tools/mark_pgdllimport.pl and it detected a few new global
variables with missing markings. See attached patch. Please point out if
any of these should not be marked or if they are special cases in some other
way. I'm Cc'ing various people involved with that new code.

FWIW, the AIO ones really don't make sense to make public - the only reason
for those variables to exists is so they can be put into an array of
callbacks. There's no way an extension could ever benefit from them. But I
guess we don't really have a way to tell mark_pgdllimport.pl that.

I did mark the internal AIO variables that maybe kinda somewhat insanely used
by an extension as PGDLLIMPORT (c.f. aio_internal.h)...

Greetings,

Andres Freund

#5Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#4)
Re: Add missing PGDLLIMPORT markings

On Wed, Apr 9, 2025 at 11:28 AM Andres Freund <andres@anarazel.de> wrote:

FWIW, the AIO ones really don't make sense to make public - the only reason
for those variables to exists is so they can be put into an array of
callbacks. There's no way an extension could ever benefit from them. But I
guess we don't really have a way to tell mark_pgdllimport.pl that.

I'm not here to say that you're wrong, but this kind of argument is
exactly why we didn't use to mark a bunch of things PGDLLIMPORT that,
as it turned out, extension developers actually wanted to use.

I don't think we should go back to the bad old days where we litigated
every case of marking something PGDLLIMPORT or not unless we have an
extremely good reason for so doing.

--
Robert Haas
EDB: http://www.enterprisedb.com

#6Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#5)
Re: Add missing PGDLLIMPORT markings

Hi,

On April 9, 2025 12:58:23 PM EDT, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Apr 9, 2025 at 11:28 AM Andres Freund <andres@anarazel.de> wrote:

FWIW, the AIO ones really don't make sense to make public - the only reason
for those variables to exists is so they can be put into an array of
callbacks. There's no way an extension could ever benefit from them. But I
guess we don't really have a way to tell mark_pgdllimport.pl that.

I'm not here to say that you're wrong, but this kind of argument is
exactly why we didn't use to mark a bunch of things PGDLLIMPORT that,
as it turned out, extension developers actually wanted to use.

I don't think we should go back to the bad old days where we litigated
every case of marking something PGDLLIMPORT or not unless we have an
extremely good reason for so doing.

Yeah, that's why I guessed that we will have to just mark these.

Fwiw, just marking everything is far from free. We prevent optimizations the compiler could make otherwise. Except of course that we have implicitly always done so in the main binary on ELF platforms, so "PGDLLIMPORT everything" kind of just removed the portability hazard.

I think eventually we ought to change the default visibility in the main binary to hidden, just like we do for extensions. Then the windows/everything else behavior difference vanishes and we actually get the benefit of being more restrictive on common platforms. I think we still ought to default to exporting symbols in such a world, just not always do so. We probably should just require all externs are either marked "server only" or "visible to extensions", never just a plain extern.

Greetings,

Andres

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

#7Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#6)
Re: Add missing PGDLLIMPORT markings

On Wed, Apr 9, 2025 at 1:10 PM Andres Freund <andres@anarazel.de> wrote:

I think eventually we ought to change the default visibility in the main binary to hidden, just like we do for extensions. Then the windows/everything else behavior difference vanishes and we actually get the benefit of being more restrictive on common platforms. I think we still ought to default to exporting symbols in such a world, just not always do so. We probably should just require all externs are either marked "server only" or "visible to extensions", never just a plain extern.

The issue we're going to run into is that a lot of extension authors
like to call things that the core developers probably think they
shouldn't. If we lock it down, we'll either be breaking a bunch of
extensions that are doing sneaky things that somebody has managed to
make work ... or we're going to be committing to an API that we don't
really want to support.

To be clear, I'm not saying you're wrong, just that it's going to be
hard to change anything without making somebody unhappy.

--
Robert Haas
EDB: http://www.enterprisedb.com

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#7)
Re: Add missing PGDLLIMPORT markings

Robert Haas <robertmhaas@gmail.com> writes:

The issue we're going to run into is that a lot of extension authors
like to call things that the core developers probably think they
shouldn't. If we lock it down, we'll either be breaking a bunch of
extensions that are doing sneaky things that somebody has managed to
make work ... or we're going to be committing to an API that we don't
really want to support.

To be clear, I'm not saying you're wrong, just that it's going to be
hard to change anything without making somebody unhappy.

Yeah. To do anything else than "export everything", we'd have to
engage in a long hard slog of negotiations over what we want to treat
as exported API and what we don't. From past experience with adding
PGDLLIMPORT markings piecemeal, that process would be never-ending,
because there would always be someone asking for access to something
else.

I don't really want to go back there. The current policy is
effectively that it's on extension developers to deal with it
when we change an API they depended on, and I'm content with that.
I don't buy that arguments like "maybe the compiler could
micro-optimize this a bit better if it weren't exported" should
drive our decision-making in this area.

regards, tom lane

#9Daniel Gustafsson
daniel@yesql.se
In reply to: Daniel Gustafsson (#2)
Re: Add missing PGDLLIMPORT markings

On 9 Apr 2025, at 13:48, Daniel Gustafsson <daniel@yesql.se> wrote:

On 9 Apr 2025, at 12:02, Peter Eisentraut <peter@eisentraut.org> wrote:

could probably do with a less generic name?

Good point, unless objected to I'll apply the attached renaming which then also
contain the PGDLLIMPORT addition.

Done.

--
Daniel Gustafsson

#10Peter Eisentraut
peter@eisentraut.org
In reply to: Peter Eisentraut (#1)
Re: Add missing PGDLLIMPORT markings

On 09.04.25 12:02, Peter Eisentraut wrote:

I ran src/tools/mark_pgdllimport.pl and it detected a few new global
variables with missing markings.  See attached patch.  Please point out
if any of these should not be marked or if they are special cases in
some other way.  I'm Cc'ing various people involved with that new code.

I have committed the remaining ones.