Re: Fix some error handling for read() and errno

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