pgsql: Remove IS_AF_UNIX macro

Started by Peter Eisentrautover 4 years ago5 messagescomitters
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

Remove IS_AF_UNIX macro

The AF_UNIX macro was being used unprotected by HAVE_UNIX_SOCKETS,
apparently since 2008. So the redirection through IS_AF_UNIX() is
apparently no longer necessary. (More generally, all supported
platforms are now HAVE_UNIX_SOCKETS, but even if there were a new
platform in the future, it seems plausible that it would define the
AF_UNIX symbol even without kernel support.) So remove the
IS_AF_UNIX() macro and make the code a bit more consistent.

Discussion: /messages/by-id/f2d26815-9832-e333-d52d-72fbc0ade896@enterprisedb.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/797129e5910144a2a937b88e145874a15b83578a

Modified Files
--------------
src/backend/libpq/hba.c | 4 ++--
src/backend/libpq/pqcomm.c | 24 ++++++++++++------------
src/backend/postmaster/postmaster.c | 4 ++--
src/include/common/ip.h | 6 ------
src/interfaces/libpq/fe-connect.c | 14 +++++++-------
5 files changed, 23 insertions(+), 29 deletions(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: pgsql: Remove IS_AF_UNIX macro

Peter Eisentraut <peter@eisentraut.org> writes:

Remove IS_AF_UNIX macro
The AF_UNIX macro was being used unprotected by HAVE_UNIX_SOCKETS,
apparently since 2008.

I hadn't looked closely at this patch, but are you referring to
this bit in ip.h?

#ifdef HAVE_UNIX_SOCKETS
#define IS_AF_UNIX(fam) ((fam) == AF_UNIX)
#else
#define IS_AF_UNIX(fam) (0)
#endif

That's by no means "unprotected": we will not try to reference
AF_UNIX unless HAVE_UNIX_SOCKETS is set. I think this change
will fail to break because we set HAVE_UNIX_SOCKETS everywhere,
but I believe it was a mistake. We might as well just nuke
all the HAVE_UNIX_SOCKETS conditional compilation if we let
this stand.

(Now, maybe we should indeed do that. I don't have much
interest in the possibility that we'll worry about such
platforms in future.)

regards, tom lane

#3Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#2)
Re: pgsql: Remove IS_AF_UNIX macro

Hi,

On 2022-02-15 10:41:44 -0500, Tom Lane wrote:

We might as well just nuke all the HAVE_UNIX_SOCKETS conditional compilation
if we let this stand.

(Now, maybe we should indeed do that. I don't have much
interest in the possibility that we'll worry about such
platforms in future.)

I'd be ok with doing that for server side stuff. But at least libpq feels like
a different story?

Greetings,

Andres Freund

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#2)
Re: pgsql: Remove IS_AF_UNIX macro

On 15.02.22 16:41, Tom Lane wrote:

Peter Eisentraut <peter@eisentraut.org> writes:

Remove IS_AF_UNIX macro
The AF_UNIX macro was being used unprotected by HAVE_UNIX_SOCKETS,
apparently since 2008.

I hadn't looked closely at this patch, but are you referring to
this bit in ip.h?

#ifdef HAVE_UNIX_SOCKETS
#define IS_AF_UNIX(fam) ((fam) == AF_UNIX)
#else
#define IS_AF_UNIX(fam) (0)
#endif

That's by no means "unprotected": we will not try to reference
AF_UNIX unless HAVE_UNIX_SOCKETS is set.

In src/backend/utils/adt/pgstatfuncs.c there is a use of AF_UNIX that
has been there unprotected by any #ifdef since about 2008.

We might as well just nuke
all the HAVE_UNIX_SOCKETS conditional compilation if we let
this stand.

(Now, maybe we should indeed do that. I don't have much
interest in the possibility that we'll worry about such
platforms in future.)

Maybe/probably. But there is a difference between platforms having the
AF_UNIX symbol (which is required by POSIX unconditionally) and
platforms actually having Unix sockets or not (which might require
different default configurations or run-time behavior), so the two
questions are not that closely connected.

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#4)
Re: pgsql: Remove IS_AF_UNIX macro

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

On 15.02.22 16:41, Tom Lane wrote:

I hadn't looked closely at this patch, but are you referring to
this bit in ip.h?

In src/backend/utils/adt/pgstatfuncs.c there is a use of AF_UNIX that
has been there unprotected by any #ifdef since about 2008.

Oh, I see it :-(. Poor coding, but it evidently no longer matters.

Maybe/probably. But there is a difference between platforms having the
AF_UNIX symbol (which is required by POSIX unconditionally) and
platforms actually having Unix sockets or not (which might require
different default configurations or run-time behavior), so the two
questions are not that closely connected.

Fair point.

regards, tom lane