Patchwork [libfortran] PR 45723 Revert previous fix

login
register
mail settings
Submitter Janne Blomqvist
Date Oct. 31, 2011, 3:51 p.m.
Message ID <CAO9iq9HUCns=WvsFkZT_=o41E8HdCSdviZZWO7ympmRTGeLpaA@mail.gmail.com>
Download mbox | patch
Permalink /patch/122875/
State New
Headers show

Comments

Janne Blomqvist - Oct. 31, 2011, 3:51 p.m.
Hi,

I'd like to revert the fix for PR 45723 that was committed previously
for the following reasons:

- Using stat("/path/to/file", ...) to infer something about an open
file descriptor is racy.

- As I argued in http://gcc.gnu.org/ml/fortran/2011-10/msg00133.html ,
I think the best approach is to do what the calling program asks as to
do, and then return failure if that doesn't work rather than trying to
fix stuff up in the library.

- In the meantime, other changes to the library have caused whatever
issue originally was behind the bug report to disappear [1]. Even so,
I'd like to remove the testcase, because I can't find any text in
POSIX which says that lseek(fd, 0, SEEK_END) for /dev/null must work,
and IMHO it's not the task of libgfortran to impose the current Linux
behavior on other systems.

[1] I haven't checked, but I recall that at some point we tried to use
buffered IO for more or less everything except terminals, and assumed
that all special files were non-seekable. Then in the buffered IO path
we returned an error if a seek on a buffered special file was
attempted. Now, however, we use buffered IO only for regular files,
for special files we do straight IO syscalls.

Regtested on x86_64-unknown-linux-gnu, Ok for trunk?

2011-10-31  Janne Blomqvist  <jb@gcc.gnu.org>

	PR libfortran/45723
	* io/open.c (new_unit): Don't check file size before attempting
	seek.

testsuite ChangeLog

2011-10-31  Janne Blomqvist  <jb@gcc.gnu.org>

	PR libfortran/45723
	* gfortran.dg/open_dev_null.F90: Remove testcase.


       u->endfile = AT_ENDFILE;
     }
jerry DeLisle - Nov. 5, 2011, 2:34 p.m.
On 10/31/2011 11:51 AM, Janne Blomqvist wrote:
> Hi,
>
> I'd like to revert the fix for PR 45723 that was committed previously
> for the following reasons:
>
> - Using stat("/path/to/file", ...) to infer something about an open
> file descriptor is racy.
>
> - As I argued in http://gcc.gnu.org/ml/fortran/2011-10/msg00133.html ,
> I think the best approach is to do what the calling program asks as to
> do, and then return failure if that doesn't work rather than trying to
> fix stuff up in the library.
>

I agree, we just can not accommodate every OS in every situation.

OK to revert.

Jerry

Patch

diff --git a/libgfortran/io/open.c b/libgfortran/io/open.c
index 0102b9c..8f969ed 100644
--- a/libgfortran/io/open.c
+++ b/libgfortran/io/open.c
@@ -554,7 +554,7 @@  new_unit (st_parameter_open *opp, gfc_unit *u,
unit_flags * flags)

   if (flags->position == POSITION_APPEND)
     {
-      if (file_size (opp->file, opp->file_len) > 0 && sseek (u->s, 0,
SEEK_END) < 0)
+      if (sseek (u->s, 0, SEEK_END) < 0)
        generate_error (&opp->common, LIBERROR_OS, NULL);