From 057674c17f6db4591da1cdf7a4a5cde3c6f98ec1 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sat, 9 Jul 2022 21:31:56 +1200 Subject: [PATCH 11/13] Remove dead pread and pwrite replacement code. pread() and pwrite() are in SUSv2 (XSI option) and SUSv4 (base), and all targeted Unix systems have them. Previously, we defined pg_pread and pg_pwrite to emulate these function with lseek() on old Unixen. The names with a pg_ prefix were a reminder of a portability hazard: they might change the current file position. That hazard is gone, so we can drop the prefixes. We still have replacement code for Windows, but the configure probes are no longer needed. Reviewed-by: Tom Lane Reviewed-by: Greg Stark Reviewed-by: Robert Haas Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com --- configure | 38 +++++--------- configure.ac | 4 +- .../pg_stat_statements/pg_stat_statements.c | 4 +- src/backend/access/heap/rewriteheap.c | 2 +- src/backend/access/transam/slru.c | 4 +- src/backend/access/transam/xlog.c | 4 +- src/backend/access/transam/xlogreader.c | 2 +- src/backend/access/transam/xlogrecovery.c | 2 +- src/backend/replication/basebackup.c | 2 +- src/backend/replication/walreceiver.c | 2 +- src/backend/storage/file/fd.c | 4 +- src/backend/utils/init/miscinit.c | 2 +- src/bin/pg_test_fsync/pg_test_fsync.c | 50 +++++++++---------- src/include/access/xlogreader.h | 4 +- src/include/pg_config.h.in | 6 --- src/include/port.h | 18 ++----- src/port/pread.c | 18 +------ src/port/preadv.c | 4 +- src/port/pwrite.c | 18 +------ src/port/pwritev.c | 4 +- src/tools/msvc/Solution.pm | 2 - 21 files changed, 68 insertions(+), 126 deletions(-) diff --git a/configure b/configure index 82443d3edb..3c7ae744ca 100755 --- a/configure +++ b/configure @@ -16752,32 +16752,6 @@ esac fi -ac_fn_c_check_func "$LINENO" "pread" "ac_cv_func_pread" -if test "x$ac_cv_func_pread" = xyes; then : - $as_echo "#define HAVE_PREAD 1" >>confdefs.h - -else - case " $LIBOBJS " in - *" pread.$ac_objext "* ) ;; - *) LIBOBJS="$LIBOBJS pread.$ac_objext" - ;; -esac - -fi - -ac_fn_c_check_func "$LINENO" "pwrite" "ac_cv_func_pwrite" -if test "x$ac_cv_func_pwrite" = xyes; then : - $as_echo "#define HAVE_PWRITE 1" >>confdefs.h - -else - case " $LIBOBJS " in - *" pwrite.$ac_objext "* ) ;; - *) LIBOBJS="$LIBOBJS pwrite.$ac_objext" - ;; -esac - -fi - ac_fn_c_check_func "$LINENO" "strlcat" "ac_cv_func_strlcat" if test "x$ac_cv_func_strlcat" = xyes; then : $as_echo "#define HAVE_STRLCAT 1" >>confdefs.h @@ -17015,6 +16989,18 @@ esac ;; esac + case " $LIBOBJS " in + *" pread.$ac_objext "* ) ;; + *) LIBOBJS="$LIBOBJS pread.$ac_objext" + ;; +esac + + case " $LIBOBJS " in + *" pwrite.$ac_objext "* ) ;; + *) LIBOBJS="$LIBOBJS pwrite.$ac_objext" + ;; +esac + case " $LIBOBJS " in *" system.$ac_objext "* ) ;; *) LIBOBJS="$LIBOBJS system.$ac_objext" diff --git a/configure.ac b/configure.ac index c1a93dff20..bc8534717d 100644 --- a/configure.ac +++ b/configure.ac @@ -1877,8 +1877,6 @@ AC_REPLACE_FUNCS(m4_normalize([ getpeereid inet_aton mkdtemp - pread - pwrite strlcat strlcpy strnlen @@ -1940,6 +1938,8 @@ if test "$PORTNAME" = "win32"; then AC_LIBOBJ(kill) AC_LIBOBJ(link) AC_LIBOBJ(open) + AC_LIBOBJ(pread) + AC_LIBOBJ(pwrite) AC_LIBOBJ(system) AC_LIBOBJ(win32env) AC_LIBOBJ(win32error) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 049da9fe6d..0d72ea212e 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -2091,9 +2091,9 @@ qtext_store(const char *query, int query_len, if (fd < 0) goto error; - if (pg_pwrite(fd, query, query_len, off) != query_len) + if (pwrite(fd, query, query_len, off) != query_len) goto error; - if (pg_pwrite(fd, "\0", 1, off + query_len) != 1) + if (pwrite(fd, "\0", 1, off + query_len) != 1) goto error; CloseTransientFile(fd); diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 197f06b5ec..9dd885d936 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -1149,7 +1149,7 @@ heap_xlog_logical_rewrite(XLogReaderState *r) /* write out tail end of mapping file (again) */ errno = 0; pgstat_report_wait_start(WAIT_EVENT_LOGICAL_REWRITE_MAPPING_WRITE); - if (pg_pwrite(fd, data, len, xlrec->offset) != len) + if (pwrite(fd, data, len, xlrec->offset) != len) { /* if write didn't set errno, assume problem is no disk space */ if (errno == 0) diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index b65cb49d7f..c9a7b97949 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -718,7 +718,7 @@ SlruPhysicalReadPage(SlruCtl ctl, int pageno, int slotno) errno = 0; pgstat_report_wait_start(WAIT_EVENT_SLRU_READ); - if (pg_pread(fd, shared->page_buffer[slotno], BLCKSZ, offset) != BLCKSZ) + if (pread(fd, shared->page_buffer[slotno], BLCKSZ, offset) != BLCKSZ) { pgstat_report_wait_end(); slru_errcause = SLRU_READ_FAILED; @@ -873,7 +873,7 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno, SlruWriteAll fdata) errno = 0; pgstat_report_wait_start(WAIT_EVENT_SLRU_WRITE); - if (pg_pwrite(fd, shared->page_buffer[slotno], BLCKSZ, offset) != BLCKSZ) + if (pwrite(fd, shared->page_buffer[slotno], BLCKSZ, offset) != BLCKSZ) { pgstat_report_wait_end(); /* if write didn't set errno, assume problem is no disk space */ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 26fbed00a4..eb5115fd5a 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -2189,7 +2189,7 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible) INSTR_TIME_SET_CURRENT(start); pgstat_report_wait_start(WAIT_EVENT_WAL_WRITE); - written = pg_pwrite(openLogFile, from, nleft, startoffset); + written = pwrite(openLogFile, from, nleft, startoffset); pgstat_report_wait_end(); /* @@ -3011,7 +3011,7 @@ XLogFileInitInternal(XLogSegNo logsegno, TimeLineID logtli, * enough. */ errno = 0; - if (pg_pwrite(fd, zbuffer.data, 1, wal_segment_size - 1) != 1) + if (pwrite(fd, zbuffer.data, 1, wal_segment_size - 1) != 1) { /* if write didn't set errno, assume no disk space */ save_errno = errno ? errno : ENOSPC; diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index f3dc4b7797..06e91547dd 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -1514,7 +1514,7 @@ WALRead(XLogReaderState *state, /* Reset errno first; eases reporting non-errno-affecting errors */ errno = 0; - readbytes = pg_pread(state->seg.ws_file, p, segbytes, (off_t) startoff); + readbytes = pread(state->seg.ws_file, p, segbytes, (off_t) startoff); #ifndef FRONTEND pgstat_report_wait_end(); diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index 5d6f1b5e46..e25e055c77 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -3210,7 +3210,7 @@ retry: readOff = targetPageOff; pgstat_report_wait_start(WAIT_EVENT_WAL_READ); - r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff); + r = pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff); if (r != XLOG_BLCKSZ) { char fname[MAXFNAMELEN]; diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index 1a569e2582..3c375b131f 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -1820,7 +1820,7 @@ basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset, int rc; pgstat_report_wait_start(WAIT_EVENT_BASEBACKUP_READ); - rc = pg_pread(fd, buf, nbytes, offset); + rc = pread(fd, buf, nbytes, offset); pgstat_report_wait_end(); if (rc < 0) diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 3d37c1fe62..8604fd4bc2 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -915,7 +915,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr, TimeLineID tli) /* OK to write the logs */ errno = 0; - byteswritten = pg_pwrite(recvFile, buf, segbytes, (off_t) startoff); + byteswritten = pwrite(recvFile, buf, segbytes, (off_t) startoff); if (byteswritten <= 0) { char xlogfname[MAXFNAMELEN]; diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 28401a8881..71e7f2e143 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -2063,7 +2063,7 @@ FileRead(File file, char *buffer, int amount, off_t offset, retry: pgstat_report_wait_start(wait_event_info); - returnCode = pg_pread(vfdP->fd, buffer, amount, offset); + returnCode = pread(vfdP->fd, buffer, amount, offset); pgstat_report_wait_end(); if (returnCode < 0) @@ -2145,7 +2145,7 @@ FileWrite(File file, char *buffer, int amount, off_t offset, retry: errno = 0; pgstat_report_wait_start(wait_event_info); - returnCode = pg_pwrite(VfdCache[file].fd, buffer, amount, offset); + returnCode = pwrite(VfdCache[file].fd, buffer, amount, offset); pgstat_report_wait_end(); /* if write didn't set errno, assume problem is no disk space */ diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 70018ca5d0..c65b596d1b 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -1429,7 +1429,7 @@ AddToDataDirLockFile(int target_line, const char *str) len = strlen(destbuffer); errno = 0; pgstat_report_wait_start(WAIT_EVENT_LOCK_FILE_ADDTODATADIR_WRITE); - if (pg_pwrite(fd, destbuffer, len, 0) != len) + if (pwrite(fd, destbuffer, len, 0) != len) { pgstat_report_wait_end(); /* if write didn't set errno, assume problem is no disk space */ diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c index 6739214eb8..8f0ef8e66b 100644 --- a/src/bin/pg_test_fsync/pg_test_fsync.c +++ b/src/bin/pg_test_fsync/pg_test_fsync.c @@ -312,10 +312,10 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (pg_pwrite(tmpfile, - buf, - XLOG_BLCKSZ, - writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); } STOP_TIMER; @@ -338,10 +338,10 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (pg_pwrite(tmpfile, - buf, - XLOG_BLCKSZ, - writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); fdatasync(tmpfile); } @@ -363,10 +363,10 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (pg_pwrite(tmpfile, - buf, - XLOG_BLCKSZ, - writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); if (fsync(tmpfile) != 0) die("fsync failed"); @@ -387,10 +387,10 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (pg_pwrite(tmpfile, - buf, - XLOG_BLCKSZ, - writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); if (pg_fsync_writethrough(tmpfile) != 0) die("fsync failed"); @@ -419,10 +419,10 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (pg_pwrite(tmpfile, - buf, - XLOG_BLCKSZ, - writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) /* * This can generate write failures if the filesystem has @@ -484,10 +484,10 @@ test_open_sync(const char *msg, int writes_size) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < 16 / writes_size; writes++) - if (pg_pwrite(tmpfile, - buf, - writes_size * 1024, - writes * writes_size * 1024) != + if (pwrite(tmpfile, + buf, + writes_size * 1024, + writes * writes_size * 1024) != writes_size * 1024) die("write failed"); } @@ -586,7 +586,7 @@ test_non_sync(void) START_TIMER; for (ops = 0; alarm_triggered == false; ops++) { - if (pg_pwrite(tmpfile, buf, XLOG_BLCKSZ, 0) != XLOG_BLCKSZ) + if (pwrite(tmpfile, buf, XLOG_BLCKSZ, 0) != XLOG_BLCKSZ) die("write failed"); } STOP_TIMER; diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 5395f155aa..87ff00feb7 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -375,11 +375,11 @@ extern bool XLogReaderValidatePageHeader(XLogReaderState *state, /* * Error information from WALRead that both backend and frontend caller can - * process. Currently only errors from pg_pread can be reported. + * process. Currently only errors from pread can be reported. */ typedef struct WALReadError { - int wre_errno; /* errno set by the last pg_pread() */ + int wre_errno; /* errno set by the last pread() */ int wre_off; /* Offset we tried to read from. */ int wre_req; /* Bytes requested to be read. */ int wre_read; /* Bytes read by the last read(). */ diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 6ea38998f8..e8633878d2 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -393,9 +393,6 @@ /* Define to 1 if you have the `ppoll' function. */ #undef HAVE_PPOLL -/* Define to 1 if you have the `pread' function. */ -#undef HAVE_PREAD - /* Define to 1 if the PS_STRINGS thing exists. */ #undef HAVE_PS_STRINGS @@ -411,9 +408,6 @@ /* Have PTHREAD_PRIO_INHERIT. */ #undef HAVE_PTHREAD_PRIO_INHERIT -/* Define to 1 if you have the `pwrite' function. */ -#undef HAVE_PWRITE - /* Define to 1 if you have the header file. */ #undef HAVE_READLINE_H diff --git a/src/include/port.h b/src/include/port.h index a8d9a64799..21c441136b 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -418,20 +418,12 @@ extern int inet_aton(const char *cp, struct in_addr *addr); #endif /* - * Windows and older Unix don't have pread(2) and pwrite(2). We have - * replacement functions, but they have slightly different semantics so we'll - * use a name with a pg_ prefix to avoid confusion. + * Windows doesn't have pread(2) and pwrite(2). We have replacement functions + * in src/port/pread.c and src/port/pwrite.c. */ -#ifdef HAVE_PREAD -#define pg_pread pread -#else -extern ssize_t pg_pread(int fd, void *buf, size_t nbyte, off_t offset); -#endif - -#ifdef HAVE_PWRITE -#define pg_pwrite pwrite -#else -extern ssize_t pg_pwrite(int fd, const void *buf, size_t nbyte, off_t offset); +#ifdef WIN32 +extern ssize_t pread(int fd, void *buf, size_t nbyte, off_t offset); +extern ssize_t pwrite(int fd, const void *buf, size_t nbyte, off_t offset); #endif /* For pg_pwritev() and pg_preadv(), see port/pg_iovec.h. */ diff --git a/src/port/pread.c b/src/port/pread.c index 491605926f..bc522d60cd 100644 --- a/src/port/pread.c +++ b/src/port/pread.c @@ -1,32 +1,24 @@ /*------------------------------------------------------------------------- * * pread.c - * Implementation of pread(2) for platforms that lack one. + * Implementation of pread(2) for Windows. * * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group * * IDENTIFICATION * src/port/pread.c * - * Note that this implementation changes the current file position, unlike - * the POSIX function, so we use the name pg_pread(). - * *------------------------------------------------------------------------- */ #include "postgres.h" -#ifdef WIN32 #include -#else -#include -#endif ssize_t -pg_pread(int fd, void *buf, size_t size, off_t offset) +pread(int fd, void *buf, size_t size, off_t offset) { -#ifdef WIN32 OVERLAPPED overlapped = {0}; HANDLE handle; DWORD result; @@ -49,10 +41,4 @@ pg_pread(int fd, void *buf, size_t size, off_t offset) } return result; -#else - if (lseek(fd, offset, SEEK_SET) < 0) - return -1; - - return read(fd, buf, size); -#endif } diff --git a/src/port/preadv.c b/src/port/preadv.c index d12e5a122b..aa7537503f 100644 --- a/src/port/preadv.c +++ b/src/port/preadv.c @@ -30,7 +30,7 @@ pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset) { #ifdef HAVE_READV if (iovcnt == 1) - return pg_pread(fd, iov[0].iov_base, iov[0].iov_len, offset); + return pread(fd, iov[0].iov_base, iov[0].iov_len, offset); if (lseek(fd, offset, SEEK_SET) < 0) return -1; return readv(fd, iov, iovcnt); @@ -40,7 +40,7 @@ pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset) for (int i = 0; i < iovcnt; ++i) { - part = pg_pread(fd, iov[i].iov_base, iov[i].iov_len, offset); + part = pread(fd, iov[i].iov_base, iov[i].iov_len, offset); if (part < 0) { if (i == 0) diff --git a/src/port/pwrite.c b/src/port/pwrite.c index eeaffacc48..a8f3f69344 100644 --- a/src/port/pwrite.c +++ b/src/port/pwrite.c @@ -1,32 +1,24 @@ /*------------------------------------------------------------------------- * * pwrite.c - * Implementation of pwrite(2) for platforms that lack one. + * Implementation of pwrite(2) for Windows. * * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group * * IDENTIFICATION * src/port/pwrite.c * - * Note that this implementation changes the current file position, unlike - * the POSIX function, so we use the name pg_pwrite(). - * *------------------------------------------------------------------------- */ #include "postgres.h" -#ifdef WIN32 #include -#else -#include -#endif ssize_t -pg_pwrite(int fd, const void *buf, size_t size, off_t offset) +pwrite(int fd, const void *buf, size_t size, off_t offset) { -#ifdef WIN32 OVERLAPPED overlapped = {0}; HANDLE handle; DWORD result; @@ -46,10 +38,4 @@ pg_pwrite(int fd, const void *buf, size_t size, off_t offset) } return result; -#else - if (lseek(fd, offset, SEEK_SET) < 0) - return -1; - - return write(fd, buf, size); -#endif } diff --git a/src/port/pwritev.c b/src/port/pwritev.c index 0bdd69fffc..cb7421381e 100644 --- a/src/port/pwritev.c +++ b/src/port/pwritev.c @@ -30,7 +30,7 @@ pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset) { #ifdef HAVE_WRITEV if (iovcnt == 1) - return pg_pwrite(fd, iov[0].iov_base, iov[0].iov_len, offset); + return pwrite(fd, iov[0].iov_base, iov[0].iov_len, offset); if (lseek(fd, offset, SEEK_SET) < 0) return -1; return writev(fd, iov, iovcnt); @@ -40,7 +40,7 @@ pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset) for (int i = 0; i < iovcnt; ++i) { - part = pg_pwrite(fd, iov[i].iov_base, iov[i].iov_len, offset); + part = pwrite(fd, iov[i].iov_base, iov[i].iov_len, offset); if (part < 0) { if (i == 0) diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index 0d4ca94e7f..811cfe1ff3 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -325,13 +325,11 @@ sub GenerateFiles HAVE_POSIX_FALLOCATE => undef, HAVE_PPC_LWARX_MUTEX_HINT => undef, HAVE_PPOLL => undef, - HAVE_PREAD => undef, HAVE_PS_STRINGS => undef, HAVE_PTHREAD => undef, HAVE_PTHREAD_BARRIER_WAIT => undef, HAVE_PTHREAD_IS_THREADED_NP => undef, HAVE_PTHREAD_PRIO_INHERIT => undef, - HAVE_PWRITE => undef, HAVE_READLINE_H => undef, HAVE_READLINE_HISTORY_H => undef, HAVE_READLINE_READLINE_H => undef, -- 2.30.2