On Fri, Aug 03, 2018 at 04:04:36PM -0400, Tom Lane wrote: > That's not good enough, because there is no reason to suppose that errno > is initially zero; in reality it'll be whatever was left over from the > last failed syscall, perhaps far distant from here. The places that do > this correctly do it like so: > > [... code ...]
Yes, I have visibly assumed that the last syscall was already failing so those conditions would not be hit. If the code gets changed again that would be a problem. > You need to go back and add the pre-clearing of errno in each of these > places, otherwise the added code is basically useless. I looked at all code paths enforcing ENOSPC on write() calls, and attached is a patch to address this issue for all of them. What do you think? I can of course get that addressed before the next set of minor releases. -- Michael
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index ed7ba181c7..85f92973c9 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -1166,6 +1166,7 @@ heap_xlog_logical_rewrite(XLogReaderState *r)
len = xlrec->num_mappings * sizeof(LogicalRewriteMappingData);
/* write out tail end of mapping file (again) */
+ errno = 0;
pgstat_report_wait_start(WAIT_EVENT_LOGICAL_REWRITE_MAPPING_WRITE);
if (write(fd, data, len) != len)
{
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 306861bb79..0ae0722794 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1669,6 +1669,7 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
errmsg("could not recreate file \"%s\": %m", path)));
/* Write content and CRC */
+ errno = 0;
pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_WRITE);
if (write(fd, content, len) != len)
{
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 822c96d1c2..bf97dcdee4 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -576,6 +576,7 @@ CheckPointReplicationOrigin(void)
tmppath)));
/* write magic */
+ errno = 0;
if ((write(tmpfd, &magic, sizeof(magic))) != sizeof(magic))
{
int save_errno = errno;
@@ -619,6 +620,7 @@ CheckPointReplicationOrigin(void)
/* make sure we only write out a commit that's persistent */
XLogFlush(local_lsn);
+ errno = 0;
if ((write(tmpfd, &disk_state, sizeof(disk_state))) !=
sizeof(disk_state))
{
@@ -641,6 +643,7 @@ CheckPointReplicationOrigin(void)
/* write out the CRC */
FIN_CRC32C(crc);
+ errno = 0;
if ((write(tmpfd, &crc, sizeof(crc))) != sizeof(crc))
{
int save_errno = errno;
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 9b55b94227..1d43a165ad 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2421,6 +2421,7 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
ondisk->size = sz;
+ errno = 0;
pgstat_report_wait_start(WAIT_EVENT_REORDER_BUFFER_WRITE);
if (write(fd, rb->outbuf, ondisk->size) != ondisk->size)
{
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 1359d9b20a..a6cd6c67d1 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1609,6 +1609,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
ereport(ERROR,
(errmsg("could not open file \"%s\": %m", path)));
+ errno = 0;
pgstat_report_wait_start(WAIT_EVENT_SNAPBUILD_WRITE);
if ((write(fd, ondisk, needed_length)) != needed_length)
{
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 6c36398058..19978d9a9e 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1275,6 +1275,7 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel)
SnapBuildOnDiskChecksummedSize);
FIN_CRC32C(cp.checksum);
+ errno = 0;
pgstat_report_wait_start(WAIT_EVENT_REPLICATION_SLOT_WRITE);
if ((write(fd, &cp, sizeof(cp))) != sizeof(cp))
{
diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index fbfee05a5a..42e3f3023a 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -122,6 +122,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
zerobuf = pg_malloc0(XLOG_BLCKSZ);
for (bytes = 0; bytes < pad_to_size; bytes += XLOG_BLCKSZ)
{
+ errno = 0;
if (write(fd, zerobuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
{
int save_errno = errno;
@@ -445,6 +446,7 @@ tar_write_compressed_data(void *buf, size_t count, bool flush)
{
size_t len = ZLIB_OUT_SIZE - tar_data->zp->avail_out;
+ errno = 0;
if (write(tar_data->fd, tar_data->zlibOut, len) != len)
{
/*
@@ -629,6 +631,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
if (!tar_data->compression)
{
+ errno = 0;
if (write(tar_data->fd, tar_data->currentfile->header, 512) != 512)
{
save_errno = errno;
@@ -829,6 +832,7 @@ tar_close(Walfile f, WalCloseMethod method)
return -1;
if (!tar_data->compression)
{
+ errno = 0;
if (write(tar_data->fd, tf->header, 512) != 512)
{
/* if write didn't set errno, assume problem is no disk space */
@@ -901,6 +905,7 @@ tar_finish(void)
MemSet(zerobuf, 0, sizeof(zerobuf));
if (!tar_data->compression)
{
+ errno = 0;
if (write(tar_data->fd, zerobuf, sizeof(zerobuf)) != sizeof(zerobuf))
{
/* if write didn't set errno, assume problem is no disk space */
@@ -933,6 +938,7 @@ tar_finish(void)
{
size_t len = ZLIB_OUT_SIZE - tar_data->zp->avail_out;
+ errno = 0;
if (write(tar_data->fd, tar_data->zlibOut, len) != len)
{
/*
signature.asc
Description: PGP signature
