Thread: pgsql: Remove check for accept() argument types
Remove check for accept() argument types This check was used to accommodate a staggering variety in particular in the type of the third argument of accept(). This is no longer of concern on currently supported systems. We can just use socklen_t in the code and put in a simple check that substitutes int for socklen_t if it's missing, to cover the few stragglers. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/3538f4c4-1886-64f2-dcff-aaad8267fb82@enterprisedb.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/ee3a1a5b636b69dde33d68c428dd56b3389a4538 Modified Files -------------- aclocal.m4 | 1 - config/ac_func_accept_argtypes.m4 | 78 ------------------------------------- configure | 82 ++++++--------------------------------- configure.ac | 2 +- src/backend/libpq/auth.c | 2 +- src/backend/libpq/pqcomm.c | 8 ++-- src/backend/postmaster/pgstat.c | 4 +- src/include/libpq/pqcomm.h | 2 +- src/include/pg_config.h.in | 15 ++----- src/include/port.h | 4 ++ src/interfaces/libpq/fe-connect.c | 2 +- src/port/getpeereid.c | 4 +- src/tools/msvc/Solution.pm | 5 +-- 13 files changed, 31 insertions(+), 178 deletions(-)
Peter Eisentraut <peter@eisentraut.org> writes:
> Remove check for accept() argument types
Early returns from the buildfarm are
gaur | 2021-11-09 16:55:58 | auth.c:3235:17: warning: pointer targets in passing argument 6 of 'recvfrom'
differin signedness
gaur | 2021-11-09 16:55:58 | pqcomm.c:722:9: warning: pointer targets in passing argument 3 of 'accept'
differin signedness
gaur | 2021-11-09 16:55:58 | pqcomm.c:743:6: warning: pointer targets in passing argument 3 of 'getsockname'
differin signedness
gaur | 2021-11-09 16:55:58 | pgstat.c:483:39: warning: pointer targets in passing argument 3 of 'getsockname'
differin signedness
gaur | 2021-11-09 16:55:58 | pgstat.c:630:9: warning: pointer targets in passing argument 5 of 'getsockopt'
differin signedness
gaur | 2021-11-09 16:55:58 | fe-connect.c:2760:11: warning: pointer targets in passing argument 5 of
'getsockopt'differ in signedness
gaur | 2021-11-09 16:55:58 | fe-connect.c:2788:9: warning: pointer targets in passing argument 3 of
'getsockname'differ in signedness
Right offhand I don't see any other animals complaining.
May I suggest that "unsigned int" would be a better choice
than "int" for socklen_t?
regards, tom lane
On 10.11.21 16:41, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: >> Remove check for accept() argument types > > Early returns from the buildfarm are > > gaur | 2021-11-09 16:55:58 | auth.c:3235:17: warning: pointer targets in passing argument 6 of 'recvfrom' differin signedness > gaur | 2021-11-09 16:55:58 | pqcomm.c:722:9: warning: pointer targets in passing argument 3 of 'accept' differin signedness > gaur | 2021-11-09 16:55:58 | pqcomm.c:743:6: warning: pointer targets in passing argument 3 of 'getsockname'differ in signedness > gaur | 2021-11-09 16:55:58 | pgstat.c:483:39: warning: pointer targets in passing argument 3 of 'getsockname'differ in signedness > gaur | 2021-11-09 16:55:58 | pgstat.c:630:9: warning: pointer targets in passing argument 5 of 'getsockopt'differ in signedness > gaur | 2021-11-09 16:55:58 | fe-connect.c:2760:11: warning: pointer targets in passing argument 5 of 'getsockopt'differ in signedness > gaur | 2021-11-09 16:55:58 | fe-connect.c:2788:9: warning: pointer targets in passing argument 3 of 'getsockname'differ in signedness > > Right offhand I don't see any other animals complaining. > May I suggest that "unsigned int" would be a better choice > than "int" for socklen_t? I have been waiting for a few more buildfarm members to finish (mainly the other AIX and HPUX instances), but they appear to be on strike right now. But based on existing results and extrapolation, it might be that gaur is actually the only one without socklen_t, so we can do whatever we want to make it happy. What does the man page say the correct type would be? size_t?
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 10.11.21 16:41, Tom Lane wrote:
>> May I suggest that "unsigned int" would be a better choice
>> than "int" for socklen_t?
> I have been waiting for a few more buildfarm members to finish (mainly
> the other AIX and HPUX instances), but they appear to be on strike right
> now.
I waited to see one of the AIX 7.1 instances report, and it does have
socklen_t. So the only old buildfarm members that any uncertainty
remains about are the HPUX 11 ones. Probably those have socklen_t;
but if they don't, given that we know HPUX 10 wants "unsigned int",
it seems certain that 11 would too. So I went ahead and pushed that
change yesterday.
> What does the man page say the correct type
> would be? size_t?
The machine's not booted up right now :-(. But I'm pretty sure we
shouldn't consider using size_t here, as it's not real clear that that
couldn't be 64 bits on any affected platforms. Your previous research
said that the desired type is 32 bits on all such platforms, so I think
that "int" is correct; we need only debate signedness.
regards, tom lane
I wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> What does the man page say the correct type
>> would be? size_t?
> The machine's not booted up right now :-(. But I'm pretty sure we
> shouldn't consider using size_t here, as it's not real clear that that
> couldn't be 64 bits on any affected platforms. Your previous research
> said that the desired type is 32 bits on all such platforms, so I think
> that "int" is correct; we need only debate signedness.
Hmm ... now that I'm in my office, I checked this and HPUX 10.20's
accept(2) man page saith
SYNOPSIS
#include <sys/socket.h>
AF_CCITT only
#include <x25/x25addrstr.h>
int accept(int s, void *addr, int *addrlen);
_XOPEN_SOURCE_EXTENDED only
int accept(int s, struct sockaddr *addr, size_t *addrlen);
gaur is using -D_XOPEN_SOURCE_EXTENDED and hence building against the
latter definition. (I found out quite a long time ago that without
_XOPEN_SOURCE_EXTENDED, this platform has a LOT of discrepancies from
SUS v2.) Further down, there's
FUTURE DIRECTION
The default behavior in this release is still the classic HP-UX BSD
Sockets, however it will be changed to X/Open Sockets in some future
release. At that time, any HP-UX BSD Sockets behavior which is
incompatible with X/Open Sockets may be obsoleted. HP customers are
advised to migrate their applications to conform to X/Open
specification (see xopen_networking(7)).
So what it looks like to me is
(1) Original BSD Sockets defined the argument as "int *addrlen".
(I confirmed this by looking in an ancient copy of Stevens'
Unix Network Programming.)
(2) X/Open thought it'd be better to use size_t.
(3) POSIX decided to resolve the conflict by inventing socklen_t.
However, SUS v2 says specifically that socklen_t "is an unsigned opaque
integral type of length of at least 32 bits". The "unsigned" part
was removed in later POSIX editions, which surprises me --- they don't
usually change the standard in the direction of less definiteness.
On the whole, I still think "unsigned int" is our best compromise.
But maybe we should use size_t and cite X/Open as authority.
regards, tom lane