Should program exit, When close() failed for O_RDONLY mode

Started by Lin, Cuipingover 5 years ago3 messages
#1Lin, Cuiping
lincuiping@cn.fujitsu.com

Hi,

I find that most of the code does not check the return value of close(), When open a file for reading(O_RDONLY).

But I find that it checks the return value of close() in code "src/bin/pg_rewind/copy_fetch.c" When open a file for reading(O_RDONLY).
And it will call pg_fatal to cause premature exit.

I think that when closing a read-only file fails, it shouid not exit the program early.It should ensure that the program execution is completed.
Like below:

・src/bin/pg_rewind/copy_fetch.c

before
--------------------------
rewind_copy_file_range
{
...
if (close(srcfd) != 0)
pg_fatal("could not close file \"%s\": %m", srcpath); }
--------------------------

after
--------------------------
rewind_copy_file_range
{
...
close(srcfd);
}
--------------------------

Regards,
--
Lin

#2Noah Misch
noah@leadboat.com
In reply to: Lin, Cuiping (#1)
Re: Should program exit, When close() failed for O_RDONLY mode

On Tue, Apr 14, 2020 at 02:32:40AM +0000, Lin, Cuiping wrote:

I find that most of the code does not check the return value of close(), When open a file for reading(O_RDONLY).

But I find that it checks the return value of close() in code "src/bin/pg_rewind/copy_fetch.c" When open a file for reading(O_RDONLY).

I think ignoring the return value is a superior style. It is less code, and
failure "can't happen."

And it will call pg_fatal to cause premature exit.

I think that when closing a read-only file fails, it shouid not exit the program early.It should ensure that the program execution is completed.

I would not say that. If close() does fail, something is badly wrong in the
program or the system running it. Though I opt not to check the return value,
if one does check it, exiting is a suitable response.

#3Michael Paquier
michael@paquier.xyz
In reply to: Noah Misch (#2)
Re: Should program exit, When close() failed for O_RDONLY mode

On Sun, May 03, 2020 at 10:18:27AM -0700, Noah Misch wrote:

I would not say that. If close() does fail, something is badly wrong in the
program or the system running it. Though I opt not to check the return value,
if one does check it, exiting is a suitable response.

FWIW, it seems to me that we have an argument for copy_fetch.c that it
can be an advantage to know if something wrong is going on
beforehand: let's remember that after running pg_rewind, the target
will be started to replay up to its consistent point.
--
Michael