Address set of issues with errno handling

System calls mixed up in error code paths are causing two issues which
several code paths have not correctly handled:
1) For write() calls, sometimes the system may return less bytes than
what has been written without errno being set.  Some paths were careful
enough to consider that case, and assumed that errno should be set to
ENOSPC, other calls missed that.
2) errno generated by a system call is overwritten by other system calls
which may succeed once an error code path is taken, causing what is
reported to the user to be incorrect.

This patch uses the brute-force approach of correcting all those code
paths.  Some refactoring could happen in the future, but this is let as
future work, which is not targeted for back-branches anyway.

Author: Michael Paquier
Reviewed-by: Ashutosh Sharma
Discussion: https://postgr.es/m/20180622061535.GD5215@paquier.xyz

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 4e0756059e850f428c341740c91e93c1de1fb873..94fd721517b04845af0687af5c8a3eb31583c56b 100644 (file)
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1237,12 +1237,17 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
     */
    if (fstat(fd, &stat))
    {
+       int         save_errno = errno;
+
        CloseTransientFile(fd);
        if (give_warnings)
+       {
+           errno = save_errno;
            ereport(WARNING,
                    (errcode_for_file_access(),
                     errmsg("could not stat two-phase state file \"%s\": %m",
                            path)));
+       }
        return NULL;
    }
 
@@ -1269,12 +1274,17 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
 
    if (read(fd, buf, stat.st_size) != stat.st_size)
    {
+       int         save_errno = errno;
+
        CloseTransientFile(fd);
        if (give_warnings)
+       {
+           errno = save_errno;
            ereport(WARNING,
                    (errcode_for_file_access(),
                     errmsg("could not read two-phase state file \"%s\": %m",
                            path)));
+       }
        pfree(buf);
        return NULL;
    }
@@ -1562,14 +1572,24 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
    /* Write content and CRC */
    if (write(fd, content, len) != len)
    {
+       int         save_errno = errno;
+
        CloseTransientFile(fd);
+
+       /* if write didn't set errno, assume problem is no disk space */
+       errno = save_errno ? save_errno : ENOSPC;
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not write two-phase state file: %m")));
    }
    if (write(fd, &statefile_crc, sizeof(pg_crc32)) != sizeof(pg_crc32))
    {
+       int         save_errno = errno;
+
        CloseTransientFile(fd);
+
+       /* if write didn't set errno, assume problem is no disk space */
+       errno = save_errno ? save_errno : ENOSPC;
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not write two-phase state file: %m")));
@@ -1581,7 +1601,10 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
     */
    if (pg_fsync(fd) != 0)
    {
+       int         save_errno = errno;
+
        CloseTransientFile(fd);
+       errno = save_errno;
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not fsync two-phase state file: %m")));

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 618d427377222c83ee4ef885d30c2f4919ffd057..f788d70e5a3381c1214858203705120dabf68a49 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2364,7 +2364,10 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
 
    if (pg_fsync(fd) != 0)
    {
+       int         save_errno = errno;
+
        close(fd);
+       errno = save_errno;
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not fsync file \"%s\": %m", tmppath)));
@@ -9777,8 +9780,10 @@ retry:
    if (lseek(readFile, (off_t) readOff, SEEK_SET) < 0)
    {
        char        fname[MAXFNAMELEN];
+       int         save_errno = errno;
 
        XLogFileName(fname, curFileTLI, readSegNo);
+       errno = save_errno;
        ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
                (errcode_for_file_access(),
                 errmsg("could not seek in log segment %s to offset %u: %m",
@@ -9789,8 +9794,10 @@ retry:
    if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
    {
        char        fname[MAXFNAMELEN];
+       int         save_errno = errno;
 
        XLogFileName(fname, curFileTLI, readSegNo);
+       errno = save_errno;
        ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
                (errcode_for_file_access(),
                 errmsg("could not read from log segment %s, offset %u: %m",

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 587d7d61c2a34cd021241bc292095b85a7bf07b8..53a5bf6d41c3b7eae5316f2102477a2a6e047f07 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -379,6 +379,8 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
            fp = AllocateFile(pathbuf, "rb");
            if (fp == NULL)
            {
+               int         save_errno = errno;
+
                /*
                 * Most likely reason for this is that the file was already
                 * removed by a checkpoint, so check for that to get a better
@@ -386,6 +388,7 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
                 */
                CheckXLogRemoved(segno, tli);
 
+               errno = save_errno;
                ereport(ERROR,
                        (errcode_for_file_access(),
                         errmsg("could not open file \"%s\": %m", pathbuf)));

diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 49ae0a598cec34f9a5f0ada5c0437ee37b5d2f9d..3e12d63530e288764743dbd53645c3cbb24e66a4 100644 (file)
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -140,6 +140,9 @@ open_walfile(XLogRecPtr startpoint, uint32 timeline, char *basedir,
    {
        if (write(f, zerobuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
        {
+           /* if write didn't set errno, assume problem is no disk space */
+           if (errno == 0)
+               errno = ENOSPC;
            fprintf(stderr,
                    _("%s: could not pad transaction log file \"%s\": %s\n"),
                    progname, fn, strerror(errno));
@@ -382,7 +385,9 @@ writeTimeLineHistoryFile(char *basedir, TimeLineID tli, char *filename,
         */
        close(fd);
        unlink(tmppath);
-       errno = save_errno;
+
+       /* if write didn't set errno, assume problem is no disk space */
+       errno = save_errno ? save_errno : ENOSPC;
 
        fprintf(stderr, _("%s: could not write timeline history file \"%s\": %s\n"),
                progname, tmppath, strerror(errno));
@@ -391,7 +396,10 @@ writeTimeLineHistoryFile(char *basedir, TimeLineID tli, char *filename,
 
    if (fsync(fd) != 0)
    {
+       int         save_errno = errno;
+
        close(fd);
+       errno = save_errno;
        fprintf(stderr, _("%s: could not fsync file \"%s\": %s\n"),
                progname, tmppath, strerror(errno));
        return false;
@@ -1125,6 +1133,12 @@ HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
                          copybuf + hdr_len + bytes_written,
                          bytes_to_write) != bytes_to_write)
                {
+                   /*
+                    * If write didn't set errno, assume problem is no disk
+                    * space.
+                    */
+                   if (errno == 0)
+                       errno = ENOSPC;
                    fprintf(stderr,
                            _("%s: could not write %u bytes to WAL file \"%s\": %s\n"),
                            progname, bytes_to_write, current_walfile_name,