pgsql: Avoid depending on non-POSIX behavior of fcntl(2).

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Avoid depending on non-POSIX behavior of fcntl(2).
Date: 2017-04-21 19:56:26
Message-ID: E1d1efK-00041v-AH@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Avoid depending on non-POSIX behavior of fcntl(2).

The POSIX standard does not say that the success return value for
fcntl(F_SETFD) and fcntl(F_SETFL) is zero; it says only that it's not -1.
We had several calls that were making the stronger assumption. Adjust
them to test specifically for -1 for strict spec compliance.

The standard further leaves open the possibility that the O_NONBLOCK
flag bit is not the only active one in F_SETFL's argument. Formally,
therefore, one ought to get the current flags with F_GETFL and store
them back with only the O_NONBLOCK bit changed when trying to change
the nonblock state. In port/noblock.c, we were doing the full pushup
in pg_set_block but not in pg_set_noblock, which is just weird. Make
both of them do it properly, since they have little business making
any assumptions about the socket they're handed. The other places
where we're issuing F_SETFL are working with FDs we just got from
pipe(2), so it's reasonable to assume the FDs' properties are all
default, so I didn't bother adding F_GETFL steps there.

Also, while pg_set_block deserves some points for trying to do things
right, somebody had decided that it'd be even better to cast fcntl's
third argument to "long". Which is completely loony, because POSIX
clearly says the third argument for an F_SETFL call is "int".

Given the lack of field complaints, these missteps apparently are not
of significance on any common platforms. But they're still wrong,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/30882.1492800880@sss.pgh.pa.us

Branch
------
REL9_3_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/899960b9fec20d55ebc8a6b7e22a90d0f65d4d17

Modified Files
--------------
contrib/pg_test_fsync/pg_test_fsync.c | 1 +
src/backend/port/unix_latch.c | 4 ++--
src/backend/postmaster/postmaster.c | 4 ++--
src/port/noblock.c | 24 ++++++++++++++++++++----
4 files changed, 25 insertions(+), 8 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2017-04-21 21:04:08 Re: [COMMITTERS] pgsql: Replication lag tracking for walsenders
Previous Message Heikki Linnakangas 2017-04-21 19:54:42 pgsql: Change the on-disk format of SCRAM verifiers to conform to RFC 5