Move NON_EXEC_STATIC from c.h

Started by Peter Eisentrautover 3 years ago4 messages
#1Peter Eisentraut
peter.eisentraut@enterprisedb.com
1 attachment(s)

Looking to tidy up c.h a bit, I think the NON_EXEC_STATIC #define
doesn't need to be known globally, and it's not related to establishing
a portable C environment, so I propose to move it to a more localized
header, such as postmaster.h, as in the attached patch.

Attachments:

0001-Move-NON_EXEC_STATIC-from-c.h-to-postmaster.h.patchtext/plain; charset=UTF-8; name=0001-Move-NON_EXEC_STATIC-from-c.h-to-postmaster.h.patchDownload
From 29f2f1a8f8111b3d232c776a145f57759131396a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 16 Aug 2022 10:56:41 +0200
Subject: [PATCH] Move NON_EXEC_STATIC from c.h to postmaster.h

It is not needed at the scope of c.h, so move it to a more localized
place.
---
 src/backend/storage/lmgr/proc.c     | 1 +
 src/include/c.h                     | 7 -------
 src/include/postmaster/postmaster.h | 6 ++++++
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 37aaab1338..448e7a2efa 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -41,6 +41,7 @@
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "postmaster/autovacuum.h"
+#include "postmaster/postmaster.h"
 #include "replication/slot.h"
 #include "replication/syncrep.h"
 #include "replication/walsender.h"
diff --git a/src/include/c.h b/src/include/c.h
index 65e91a6b89..17d1cc28e3 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1348,13 +1348,6 @@ typedef intptr_t sigjmp_buf[5];
 #endif							/* __MINGW64__ */
 #endif							/* WIN32 */
 
-/* EXEC_BACKEND defines */
-#ifdef EXEC_BACKEND
-#define NON_EXEC_STATIC
-#else
-#define NON_EXEC_STATIC static
-#endif
-
 /* /port compatibility functions */
 #include "port.h"
 
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 90e333ccd2..03c624fb1b 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -55,6 +55,12 @@ extern int	MaxLivePostmasterChildren(void);
 
 extern bool PostmasterMarkPIDForWorkerNotify(int);
 
+#ifdef EXEC_BACKEND
+#define NON_EXEC_STATIC
+#else
+#define NON_EXEC_STATIC static
+#endif
+
 #ifdef EXEC_BACKEND
 extern pid_t postmaster_forkexec(int argc, char *argv[]);
 extern void SubPostmasterMain(int argc, char *argv[]) pg_attribute_noreturn();
-- 
2.37.1

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: Move NON_EXEC_STATIC from c.h

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Looking to tidy up c.h a bit, I think the NON_EXEC_STATIC #define
doesn't need to be known globally, and it's not related to establishing
a portable C environment, so I propose to move it to a more localized
header, such as postmaster.h, as in the attached patch.

Hmm, postgres.h seems like a better choice, since in principle any
backend file might need this. This arrangement could require
postmaster.h to be included just for this macro.

Also, the macro was severely underdocumented already, and I don't
find "no comment at all" to be better. Can't we afford a couple
of lines of explanation?

regards, tom lane

#3Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Tom Lane (#2)
1 attachment(s)
Re: Move NON_EXEC_STATIC from c.h

On 16.08.22 15:50, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Looking to tidy up c.h a bit, I think the NON_EXEC_STATIC #define
doesn't need to be known globally, and it's not related to establishing
a portable C environment, so I propose to move it to a more localized
header, such as postmaster.h, as in the attached patch.

Hmm, postgres.h seems like a better choice, since in principle any
backend file might need this. This arrangement could require
postmaster.h to be included just for this macro.

I picked postmaster.h because the other side of the code, where the
no-longer-static symbols are used, is in postmaster.c. But postgres.h
is also ok.

Also, the macro was severely underdocumented already, and I don't
find "no comment at all" to be better. Can't we afford a couple
of lines of explanation?

Here is a new patch with more comments.

Attachments:

v2-0001-Move-NON_EXEC_STATIC-from-c.h-to-postgres.h.patchtext/plain; charset=UTF-8; name=v2-0001-Move-NON_EXEC_STATIC-from-c.h-to-postgres.h.patchDownload
From 06f30dcf46ed4f89e0c9c5e67615a5c9ad492988 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 23 Aug 2022 20:24:21 +0200
Subject: [PATCH v2] Move NON_EXEC_STATIC from c.h to postgres.h

It is not needed at the scope of c.h, only in backend code.

Discussion: https://www.postgresql.org/message-id/flat/a6a6b48e-ca0a-b58d-18de-98e40d94b842%40enterprisedb.com
---
 src/include/c.h        |  7 -------
 src/include/postgres.h | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/include/c.h b/src/include/c.h
index dfc366b026..aad9554de6 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1338,13 +1338,6 @@ typedef intptr_t sigjmp_buf[5];
 #endif							/* __MINGW64__ */
 #endif							/* WIN32 */
 
-/* EXEC_BACKEND defines */
-#ifdef EXEC_BACKEND
-#define NON_EXEC_STATIC
-#else
-#define NON_EXEC_STATIC static
-#endif
-
 /* /port compatibility functions */
 #include "port.h"
 
diff --git a/src/include/postgres.h b/src/include/postgres.h
index 31358110dc..13903fa022 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -25,6 +25,7 @@
  *	  -------	------------------------------------------------
  *		1)		variable-length datatypes (TOAST support)
  *		2)		Datum type + support macros
+ *		3)		miscellaneous
  *
  *	 NOTES
  *
@@ -805,4 +806,23 @@ extern Datum Float8GetDatum(float8 X);
 #define Float8GetDatumFast(X) PointerGetDatum(&(X))
 #endif
 
+
+/* ----------------------------------------------------------------
+ *				Section 3:	miscellaneous
+ * ----------------------------------------------------------------
+ */
+
+/*
+ * NON_EXEC_STATIC: It's sometimes useful to define a variable or function
+ * that is normally static but extern when using EXEC_BACKEND (see
+ * pg_config_manual.h).  There would then typically be some code in
+ * postmaster.c that uses those extern symbols to transfer state between
+ * processes or do whatever other things it needs to do in EXEC_BACKEND mode.
+ */
+#ifdef EXEC_BACKEND
+#define NON_EXEC_STATIC
+#else
+#define NON_EXEC_STATIC static
+#endif
+
 #endif							/* POSTGRES_H */
-- 
2.37.1

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#3)
Re: Move NON_EXEC_STATIC from c.h

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Here is a new patch with more comments.

LGTM

regards, tom lane