Обсуждение: Rationalizing declarations of src/common/ variables
We've been burnt by this issue repeatedly (cf c2d1eea9e, d025cf88b,
11b500072) so I think it's time to try to formalize and document
what to do to export a variable from src/common/ or src/port/.
Here's a draft patch. I'm not in love with the name "PGDLLIMPORT_FE"
and would welcome better ideas.
regards, tom lane
diff --git a/src/include/c.h b/src/include/c.h
index c8ede08273..e124e02d62 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1312,10 +1312,34 @@ extern long long strtoll(const char *str, char **endptr, int base);
extern unsigned long long strtoull(const char *str, char **endptr, int base);
#endif
-/* no special DLL markers on most ports */
+/*
+ * Use "extern PGDLLIMPORT ..." to declare variables that are defined in
+ * the core backend and need to be accessible by loadable modules.
+ * No special marking is required on most ports.
+ */
#ifndef PGDLLIMPORT
#define PGDLLIMPORT
#endif
+
+/*
+ * Use "extern PGDLLIMPORT_FE ..." to declare variables that are defined in
+ * common frontend/backend libraries (src/common/ or src/port/). In the
+ * server, these are defined in the core backend and need to be accessible by
+ * loadable modules. In frontend programs, these are defined locally and need
+ * no marking. In any case, no special marking is required on most ports.
+ */
+#ifndef FRONTEND
+#define PGDLLIMPORT_FE PGDLLIMPORT
+#else
+#define PGDLLIMPORT_FE
+#endif
+
+/*
+ * Use "extern PGDLLEXPORT ..." to declare functions that are defined in
+ * loadable modules and need to be callable by the core backend. Usually,
+ * this is not necessary because our build process automatically exports
+ * such symbols.
+ */
#ifndef PGDLLEXPORT
#define PGDLLEXPORT
#endif
diff --git a/src/include/common/keywords.h b/src/include/common/keywords.h
index 19e4eda8f9..54e40ac66c 100644
--- a/src/include/common/keywords.h
+++ b/src/include/common/keywords.h
@@ -22,14 +22,8 @@
#define TYPE_FUNC_NAME_KEYWORD 2
#define RESERVED_KEYWORD 3
-#ifndef FRONTEND
-extern PGDLLIMPORT const ScanKeywordList ScanKeywords;
-extern PGDLLIMPORT const uint8 ScanKeywordCategories[];
-extern PGDLLIMPORT const bool ScanKeywordBareLabel[];
-#else
-extern const ScanKeywordList ScanKeywords;
-extern const uint8 ScanKeywordCategories[];
-extern const bool ScanKeywordBareLabel[];
-#endif
+extern PGDLLIMPORT_FE const ScanKeywordList ScanKeywords;
+extern PGDLLIMPORT_FE const uint8 ScanKeywordCategories[];
+extern PGDLLIMPORT_FE const bool ScanKeywordBareLabel[];
#endif /* KEYWORDS_H */
diff --git a/src/include/common/pg_prng.h b/src/include/common/pg_prng.h
index e4df5165d7..a07214fd11 100644
--- a/src/include/common/pg_prng.h
+++ b/src/include/common/pg_prng.h
@@ -26,11 +26,7 @@ typedef struct pg_prng_state
* Callers not needing local PRNG series may use this global state vector,
* after initializing it with one of the pg_prng_...seed functions.
*/
-#ifndef FRONTEND
-extern PGDLLIMPORT pg_prng_state pg_global_prng_state;
-#else
-extern pg_prng_state pg_global_prng_state;
-#endif
+extern PGDLLIMPORT_FE pg_prng_state pg_global_prng_state;
extern void pg_prng_seed(pg_prng_state *state, uint64 seed);
extern void pg_prng_fseed(pg_prng_state *state, double fseed);
diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 7dd7fef4f7..0cdbfe7a14 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -13,15 +13,9 @@
#ifndef PG_BITUTILS_H
#define PG_BITUTILS_H
-#ifndef FRONTEND
-extern PGDLLIMPORT const uint8 pg_leftmost_one_pos[256];
-extern PGDLLIMPORT const uint8 pg_rightmost_one_pos[256];
-extern PGDLLIMPORT const uint8 pg_number_of_ones[256];
-#else
-extern const uint8 pg_leftmost_one_pos[256];
-extern const uint8 pg_rightmost_one_pos[256];
-extern const uint8 pg_number_of_ones[256];
-#endif
+extern PGDLLIMPORT_FE const uint8 pg_leftmost_one_pos[256];
+extern PGDLLIMPORT_FE const uint8 pg_rightmost_one_pos[256];
+extern PGDLLIMPORT_FE const uint8 pg_number_of_ones[256];
/*
* pg_leftmost_one_pos32
On Mon, Nov 29, 2021 at 11:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > We've been burnt by this issue repeatedly (cf c2d1eea9e, d025cf88b, > 11b500072) so I think it's time to try to formalize and document > what to do to export a variable from src/common/ or src/port/. +1 to document it. > Here's a draft patch. I'm not in love with the name "PGDLLIMPORT_FE" > and would welcome better ideas. How about PGDLLIMPORT_FE_BE which represents the macro to be used for variables/functions common to both frontend and backend? Otherwise, PGDLLIMPORT_COMM/PGDLLIMPORT_COMMON or PGDLLIMPORT_2 or PGDLLIMPORT_PORT? We have some of the #defines with "FeBe": /* * prototypes for functions in pqcomm.c */ extern WaitEventSet *FeBeWaitSet; #define FeBeWaitSetSocketPos 0 #define FeBeWaitSetLatchPos 1 Regards, Bharath Rupireddy.
On Mon, Nov 29, 2021 at 12:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Here's a draft patch. I'm not in love with the name "PGDLLIMPORT_FE" > and would welcome better ideas. What's the value of introducing PGDLLIMPORT_FE? I mean suppose we just make PGDLLIMPORT expand to nothing in front-end code. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> What's the value of introducing PGDLLIMPORT_FE? I mean suppose we just
> make PGDLLIMPORT expand to nothing in front-end code.
Hmm ... fair question. It feels like that risks breaking something,
but offhand I can't see what, as long as we're certain that FRONTEND
is set correctly in every compile.
regards, tom lane
On Mon, Nov 29, 2021 at 9:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > What's the value of introducing PGDLLIMPORT_FE? I mean suppose we just > > make PGDLLIMPORT expand to nothing in front-end code. > > Hmm ... fair question. It feels like that risks breaking something, > but offhand I can't see what, as long as we're certain that FRONTEND > is set correctly in every compile. If it isn't, your way might go wrong too, since it depends on FRONTEND being set correctly at least at the point when the PGDLLIMPORT_FE macro is defined. But that is not to say that I think everything is in great shape in this area. In a perfect world, I think the only '#define FRONTEND 1' in the backend would be in postgres_fe.h, but we have it in 5 other places too, 3 of which include a comment saying that it's an "ugly hack". Until somebody cleans that mess up, we have at least three cases to worry about: backend code that includes "postgres.h", front code that includes "postgres-fe.h", and frbontackend code that first does #define FRONTEND 1 and then includes "postgres.h" anyway. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 29, 2021 at 9:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> What's the value of introducing PGDLLIMPORT_FE? I mean suppose we just
>>> make PGDLLIMPORT expand to nothing in front-end code.
>> Hmm ... fair question. It feels like that risks breaking something,
>> but offhand I can't see what, as long as we're certain that FRONTEND
>> is set correctly in every compile.
> If it isn't, your way might go wrong too, since it depends on FRONTEND
> being set correctly at least at the point when the PGDLLIMPORT_FE
> macro is defined.
Either of these ways would require that FRONTEND is already set correctly
when c.h is read. But all of the hacks you mention do ensure that.
regards, tom lane
On Mon, Nov 29, 2021 at 10:03 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Either of these ways would require that FRONTEND is already set correctly > when c.h is read. But all of the hacks you mention do ensure that. Yeah. Are you aware of any other, worse hacks? -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 29, 2021 at 10:03 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Either of these ways would require that FRONTEND is already set correctly
>> when c.h is read. But all of the hacks you mention do ensure that.
> Yeah. Are you aware of any other, worse hacks?
Worse than which? Anyway, I pushed a patch based on your suggestion;
we'll soon see if the Windows BF members like it.
regards, tom lane