Rationalizing declarations of src/common/ variables

Started by Tom Laneover 4 years ago8 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

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

Attachments:

improve-variable-export-declarations-1.patchtext/x-diff; charset=us-ascii; name=improve-variable-export-declarations-1.patchDownload+32-24
#2Bharath Rupireddy
bharath.rupireddyforpostgres@gmail.com
In reply to: Tom Lane (#1)
Re: Rationalizing declarations of src/common/ variables

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.

#3Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#1)
Re: Rationalizing declarations of src/common/ variables

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#3)
Re: Rationalizing declarations of src/common/ variables

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

#5Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#4)
Re: Rationalizing declarations of src/common/ variables

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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#5)
Re: Rationalizing declarations of src/common/ variables

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

#7Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#6)
Re: Rationalizing declarations of src/common/ variables

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

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#7)
Re: Rationalizing declarations of src/common/ variables

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