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

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

From
"Lin, Cuiping"
Date:
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
programexecution 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






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

From
Noah Misch
Date:
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
programexecution 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.



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

From
Michael Paquier
Date:
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

Attachment