diff mbox

[libgfortran] PR45723 opening /dev/null for appending writes fails

Message ID 4C981E53.1080608@frontier.com
State New
Headers show

Commit Message

Jerry DeLisle Sept. 21, 2010, 2:54 a.m. UTC
Hi,

This patch is simple. Test case attached.

Regression tested on x86-64.

OK for trunk?

Jerry

2010-09-20  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/45723
	* io/open.c (new_unit): On POSITION_APPEND don't seek if file length is
	zero.

! { dg-do run }
! PR45723 opening /dev/null for appending writes fails
logical :: thefile
inquire(file="/dev/null",exist=thefile)
if (thefile) then
  open(unit=7,file="/dev/null",position="append")
  close(7)
endif
end

Comments

Steve Kargl Sept. 21, 2010, 4:56 a.m. UTC | #1
On Mon, Sep 20, 2010 at 07:54:11PM -0700, Jerry DeLisle wrote:
> Hi,
> 
> This patch is simple. Test case attached.
> 
> Regression tested on x86-64.
> 
> OK for trunk?
> 

Is the file size guaranteed to be 0 for all 
non-seekable files on all operating systems
supported by gfortran?  If someone is stupid
enough to open /dev/null for append, then
he should be prepared to suffer the consequence.
Jerry DeLisle Sept. 22, 2010, 1:30 a.m. UTC | #2
On 09/20/2010 09:56 PM, Steve Kargl wrote:
> On Mon, Sep 20, 2010 at 07:54:11PM -0700, Jerry DeLisle wrote:
>> Hi,
>>
>> This patch is simple. Test case attached.
>>
>> Regression tested on x86-64.
>>
>> OK for trunk?
>>
>
> Is the file size guaranteed to be 0 for all
> non-seekable files on all operating systems
> supported by gfortran?  If someone is stupid
> enough to open /dev/null for append, then
> he should be prepared to suffer the consequence.
>

You will notice the patch does nothing that presumes anything about the file, 
seekable or not.

All that patch is doing is not seeking if the length is zero or less. The 
filesize function being used will return a value less then zero for a few error 
conditions but in this case I simply chose to ignore it.  So, if the file length 
happens to be zero, there is no need to seek.  If that happens to work for 
/dev/null on some systems, no harm done.

 From the typical man page:

"Data written to a null or zero special file is discarded." So my thinking, it 
should not matter "how" the file is opened if it is going to be discarded 
anyway.  I suppose a user may find it useful during development of a program to 
write data to /dev/null, for example during trial runs where you would normally 
append, but for testing, do not want to keep growing the file.  Who knows, not 
my place to question why someone might want to do this.  It was suggested by one 
of our more valued testors.  That was enough for me.

OK for trunk.  :)

Jerry
Steve Kargl Sept. 22, 2010, 1:50 a.m. UTC | #3
On Tue, Sep 21, 2010 at 06:30:55PM -0700, Jerry DeLisle wrote:
> On 09/20/2010 09:56 PM, Steve Kargl wrote:
> >On Mon, Sep 20, 2010 at 07:54:11PM -0700, Jerry DeLisle wrote:
> >>Hi,
> >>
> >>This patch is simple. Test case attached.
> >>
> >>Regression tested on x86-64.
> >>
> >>OK for trunk?
> >>
> >
> >Is the file size guaranteed to be 0 for all
> >non-seekable files on all operating systems
> >supported by gfortran?  If someone is stupid
> >enough to open /dev/null for append, then
> >he should be prepared to suffer the consequence.
> >
> 
> You will notice the patch does nothing that presumes anything about the 
> file, seekable or not.
> 
> All that patch is doing is not seeking if the length is zero or less. The 
> filesize function being used will return a value less then zero for a few 
> error conditions but in this case I simply chose to ignore it.  So, if the 
> file length happens to be zero, there is no need to seek.  If that happens 
> to work for /dev/null on some systems, no harm done.
> 
> From the typical man page:
> 
> "Data written to a null or zero special file is discarded." So my thinking, 
> it should not matter "how" the file is opened if it is going to be 
> discarded anyway.  I suppose a user may find it useful during development 
> of a program to write data to /dev/null, for example during trial runs 
> where you would normally append, but for testing, do not want to keep 
> growing the file.  Who knows, not my place to question why someone might 
> want to do this.  It was suggested by one of our more valued testors.  That 
> was enough for me.
> 
> OK for trunk.  :)
> 

Sure.  Clearly, if the current filesize is 0, then 
opening for append is semantically the same as simply
opening the file for write.
diff mbox

Patch

Index: open.c
===================================================================
--- open.c	(revision 164472)
+++ open.c	(working copy)
@@ -555,7 +555,7 @@  new_unit (st_parameter_open *opp, gfc_unit *u, uni
 
   if (flags->position == POSITION_APPEND)
     {
-      if (sseek (u->s, 0, SEEK_END) < 0)
+      if (file_size (opp->file, opp->file_len) > 0 && sseek (u->s, 0, SEEK_END) < 0)
 	generate_error (&opp->common, LIBERROR_OS, NULL);
       u->endfile = AT_ENDFILE;
     }