Re: Fix some error handling for read() and errno - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Fix some error handling for read() and errno
Date
Msg-id 20180523000040.GA3461@paquier.xyz
Whole thread Raw
In response to Re: Fix some error handling for read() and errno  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Responses Re: Fix some error handling for read() and errno  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
List pgsql-hackers
On Tue, May 22, 2018 at 04:51:00PM +0900, Kyotaro HORIGUCHI wrote:
> I see the same issue in snapbuild.c(4 places).
>
> | readBytes = read(fd, &ondisk, SnapBuildOnDiskConstantSize);
> | pgstat_report_wait_end();
> | if (readBytes != SnapBuildOnDiskConstantSize)
> | {
> |   CloseTransientFile(fd);
> |   ereport(ERROR,
> |       (errcode_for_file_access(),
> |        errmsg("could not read file \"%s\", read %d of %d: %m",
> |           path, readBytes, (int) SnapBuildOnDiskConstantSize)));
> | }

Four times the same pattern, which also bloat errno when closing the
file descriptor.  I did not catch those.

> and walsender.c (2 places)
>
> |   if (nread <= 0)
> |     ereport(ERROR,
> |         (errcode_for_file_access(),
> |          errmsg("could not read file \"%s\": %m",
> |             path)));

Those two ones I saw, but I was not sure if it is worth the complication
to error on an empty file.  We could do something like the attached which
would be an improvement in readability?

> and pg_receivewal.c
>
> | if (read(fd, (char *) buf, sizeof(buf)) != sizeof(buf))
> | {
> |   fprintf(stderr, _("%s: could not read compressed file \"%s\": %s\n"),
> |       progname, fullpath, strerror(errno));

Okay.

> pg_waldump.c
>
> | if (readbytes <= 0)
> ...
> |   fatal_error("could not read from log file %s, offset %u, length %d: %s",
> |         fname, sendOff, segbytes, strerror(err));
>
>
> A bit different issue, but in pg_waldump.c, search_directory can
> check uninitialized errno when read returns a non-zero value.

Yeah, the error message could be improved as well if the result is an
empty file.

Updated patch is attached.  Thanks for your review.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Matthew Stickney
Date:
Subject: Re: [PATCH] (Windows) psql echoes password when reading from pipe
Next
From: Michael Paquier
Date:
Subject: Re: PostgreSQL “tuple already updated by self”