From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | pgsql-committers(at)lists(dot)postgresql(dot)org |
Subject: | Re: pgsql: Address set of issues with errno handling |
Date: | 2018-08-03 20:04:36 |
Message-ID: | 31797.1533326676@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
Michael Paquier <michael(at)paquier(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2018-08-03 20:36:48 | pgsql: Fix pg_replication_slot example output |
Previous Message | Tom Lane | 2018-08-03 16:21:02 | pgsql: Change libpq's internal uses of PQhost() to inspect host field d |