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