From 9d543f0cf5806625e5d823c18cf8a4d390ec7263 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sat, 9 Jul 2022 21:31:56 +1200 Subject: [PATCH 1/6] Remove dead pg_pread and pg_pwrite replacement code. pread and pwrite were standardised in POSIX-1:2001. 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. Since the replacement code for Windows doesn't suffer from the position problem, we might as well start using the POSIX names directly. --- .../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/port.h | 17 +++---- src/port/pread.c | 18 +------ src/port/preadv.c | 4 +- src/port/pwrite.c | 18 +------ src/port/pwritev.c | 4 +- 17 files changed, 55 insertions(+), 88 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 4acfddcdb8..e8a7666ed9 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -2096,9 +2096,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 b809a2152c..80fe2ca5ab 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 95440013c0..c02b583976 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -1827,7 +1827,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 f904f60c08..07bd41aaa8 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -2067,7 +2067,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) @@ -2149,7 +2149,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 eb43b2c5e5..bd973ba613 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 f7bc199a30..be8effe16b 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/port.h b/src/include/port.h index 12c05b5d9f..68101f7b16 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -421,20 +421,15 @@ 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); +#ifndef HAVE_PREAD +extern ssize_t 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); +#ifndef HAVE_PWRITE +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) -- 2.36.1