pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Address set of issues with errno handling
Date: Fri, 03 Aug 2018 16:04:36 -0400
Message-ID: <31797.1533326676@sss.pgh.pa.us> (raw)
In-Reply-To: <E1fXHAd-0004Qv-8y@gemulon.postgresql.org>
References: <E1fXHAd-0004Qv-8y@gemulon.postgresql.org>

Michael Paquier <michael@paquier.xyz> writes:
> Address set of issues with errno handling

I happened to look at this patch while working up the release notes,
and I don't think it's right at all for this aspect:

> 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.

AFAICS, what you did about that was like this:

@@ -816,7 +828,12 @@ tar_close(Walfile f, WalCloseMethod method)
    if (!tar_data->compression)
    {
        if (write(tar_data->fd, tf->header, 512) != 512)
+       {
+           /* if write didn't set errno, assume problem is no disk space */
+           if (errno == 0)
+               errno = ENOSPC;
            return -1;
+       }
    }
 #ifdef HAVE_LIBZ
    else

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:

        errno = 0;
        if (write(...) != expected-length)
        {
            /* if write didn't set errno, assume problem is no disk space */
            if (errno == 0)
                errno = ENOSPC;
            ...
        }

You need to go back and add the pre-clearing of errno in each of these
places, otherwise the added code is basically useless.

			regards, tom lane




view thread (11+ messages)  latest in thread

Message-ID: <31797.1533326676@sss.pgh.pa.us>
Permalink:  ../31797.1533326676@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/31797.1533326676@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Address set of issues with errno handling
  In-Reply-To: <31797.1533326676@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox