I wrote: > Actually, after re-reading the thread that led to 06843df4a [1], > I think a better idea is to introduce some macro magic to force > frontend clients to use libpgport's version of pqsignal() instead > of the one from libpq. We mustn't change the real name of libpq's > version, but I think we could get away with that for libpgport.
After studying this more, I think what we should do in HEAD is even more aggressive: let's make the real name of port/pqsignal.c's function be either pqsignal_fe in frontend, or pqsignal_be in backend. This positively ensures that there's no collision with libpq's export, and it seems like a good idea for the additional reason that the frontend and backend versions are not really interchangeable. However, we can't get away with that in released branches because it'd be an ABI break for any outside code that calls pqsignal. What I propose doing in the released branches is what's shown in the attached patch for v17: rename port/pqsignal.c's function to pqsignal_fe in frontend, but leave it as pqsignal in the backend. Leaving it as pqsignal in the backend preserves ABI for extension modules, at the cost that it's not entirely clear which pqsignal an extension that's also linked with libpq will get. But that hazard affects none of our code, and it existed already for any third-party extensions that call pqsignal. In principle using "pqsignal_fe" in frontend creates an ABI hazard for outside users of libpgport.a. But I think it's not really a problem, because we don't support that as a shared library. As long as people build with headers and .a files from the same minor release, they'll be OK. BTW, this decouples legacy-pqsignal.c from pqsignal.c enough that we could now do what's contemplated in the comments from 3b00fdba9: simplify that version by making it return void, or perhaps better just a true/false success report. I've not touched that point here, though. regards, tom lane
diff --git a/src/include/port.h b/src/include/port.h index 818b7c7bae..f0e28ce5c5 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -513,7 +513,12 @@ extern int pg_check_dir(const char *dir); /* port/pgmkdirp.c */ extern int pg_mkdir_p(char *path, int omode); -/* port/pqsignal.c */ +/* port/pqsignal.c (see also interfaces/libpq/legacy-pqsignal.c) */ +#ifdef FRONTEND +#define pqsignal pqsignal_fe +#else +#define pqsignal pqsignal_be +#endif typedef void (*pqsigfunc) (SIGNAL_ARGS); extern pqsigfunc pqsignal(int signo, pqsigfunc func); diff --git a/src/interfaces/libpq/legacy-pqsignal.c b/src/interfaces/libpq/legacy-pqsignal.c index 0864888562..01977b4c81 100644 --- a/src/interfaces/libpq/legacy-pqsignal.c +++ b/src/interfaces/libpq/legacy-pqsignal.c @@ -28,8 +28,16 @@ * with the semantics it had in 9.2; in particular, this has different * behavior for SIGALRM than the version in src/port/pqsignal.c. * - * libpq itself does not use this. + * libpq itself does not use this, nor does anything else in our code. + * + * src/include/port.h #define's pqsignal as pqsignal_fe or pqsignal_be, + * but here we want to export just plain "pqsignal". We can't rely on + * port.h's extern declaration either. (The point of the #define's + * is to ensure that no in-tree code accidentally calls this version.) */ +#undef pqsignal +extern pqsigfunc pqsignal(int signo, pqsigfunc func); + pqsigfunc pqsignal(int signo, pqsigfunc func) { diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c index d328a27279..f707b1c54e 100644 --- a/src/port/pqsignal.c +++ b/src/port/pqsignal.c @@ -123,6 +123,10 @@ wrapper_handler(SIGNAL_ARGS) * function instead of providing potentially-bogus return values. * Unfortunately, that requires modifying the pqsignal() in legacy-pqsignal.c, * which in turn requires an SONAME bump, which is probably not worth it. + * + * Note: the actual name of this function is either pqsignal_fe when + * compiled with -DFRONTEND, or pqsignal_be when compiled without. + * This is to avoid a name collision with libpq's legacy-pqsignal.c. */ pqsigfunc pqsignal(int signo, pqsigfunc func)
diff --git a/src/include/port.h b/src/include/port.h index 4d2d911cb6..acc39deac4 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -515,7 +515,10 @@ extern int pg_check_dir(const char *dir); /* port/pgmkdirp.c */ extern int pg_mkdir_p(char *path, int omode); -/* port/pqsignal.c */ +/* port/pqsignal.c (see also interfaces/libpq/legacy-pqsignal.c) */ +#ifdef FRONTEND +#define pqsignal pqsignal_fe +#endif typedef void (*pqsigfunc) (SIGNAL_ARGS); extern pqsigfunc pqsignal(int signo, pqsigfunc func); diff --git a/src/interfaces/libpq/legacy-pqsignal.c b/src/interfaces/libpq/legacy-pqsignal.c index e8c716ad0f..49755e8acd 100644 --- a/src/interfaces/libpq/legacy-pqsignal.c +++ b/src/interfaces/libpq/legacy-pqsignal.c @@ -28,8 +28,16 @@ * with the semantics it had in 9.2; in particular, this has different * behavior for SIGALRM than the version in src/port/pqsignal.c. * - * libpq itself does not use this. + * libpq itself does not use this, nor does anything else in our code. + * + * src/include/port.h will #define pqsignal as pqsignal_fe, + * but here we want to export just plain "pqsignal". We can't rely on + * port.h's extern declaration either. (The point of the #define + * is to ensure that no in-tree code accidentally calls this version.) */ +#undef pqsignal +extern pqsigfunc pqsignal(int signo, pqsigfunc func); + pqsigfunc pqsignal(int signo, pqsigfunc func) { diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c index bdaa9f10c8..844e37c827 100644 --- a/src/port/pqsignal.c +++ b/src/port/pqsignal.c @@ -123,6 +123,10 @@ wrapper_handler(SIGNAL_ARGS) * function instead of providing potentially-bogus return values. * Unfortunately, that requires modifying the pqsignal() in legacy-pqsignal.c, * which in turn requires an SONAME bump, which is probably not worth it. + * + * Note: the actual name of this function is either pqsignal_fe when + * compiled with -DFRONTEND, or pqsignal when compiled without. + * This is to avoid a name collision with libpq's legacy-pqsignal.c. */ pqsigfunc pqsignal(int signo, pqsigfunc func)