[patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

I will commit the following patch under the obvious and simple rule to trunk.

This fixes the latest test case for pr31052 and does not break the SPEC 
benchmark.  Namelist fails without this patch which avoids the call to 
test_endfile.  SPEC fails without the call to test_endfile.

I will, as I have time, work up a suitable testcase.  (see the pr for the test 
case involved)  I also will try to get the publicly available six.f program to 
compile and run and see if I can determine any better approach to all this.  For 
now it fixes the problem.

Regards,

Jerry

Index: io/transfer.c
===================================================================
*** io/transfer.c       (revision 123253)
--- io/transfer.c       (working copy)
*************** next_record_r (st_parameter_dt *dtp)
*** 2228,2234 ****
         break;
       }

!   if (dtp->u.p.current_unit->flags.access == ACCESS_SEQUENTIAL)
       test_endfile (dtp->u.p.current_unit);
   }

--- 2228,2235 ----
         break;
       }

!   if (dtp->u.p.current_unit->flags.access == ACCESS_SEQUENTIAL
!       && !dtp->u.p.namelist_mode)
       test_endfile (dtp->u.p.current_unit);
   }

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[FX Coudert]

> 2007-03-30  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
>
>     PR libgfortran/31052
>     * io/file_position (st_rewind): Remove use of
>     test_endfile. Don't seek if already at 0 position.  Use new  
> is_special
>     function to set endfile state.
>     * io/open.c (test_endfile): Delete this function.
>     (edit_modes): Delete call to test_endfile.
>     (new_unit): Likewise.
>     * io/io.h: Delete prototype for test_endfile.  Add prototype
>     for is_special.
>     * io/transfer.c (next_record_r): Remove use of test_endfile.
>     (st_read):  Add test for end file condition and adjust status.

OK for mainline.

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

:ADDPATCH fortran:

I forgot the little detail above.
Jerry DeLisle wrote:
> Jerry DeLisle wrote:
>>
>> Here is a final installment with test case.  I eliminated the 
>> test_endfile function.  Then I added a new function is_special () that 
>> returns true if the special file bit is set in the stream structure.
>>
> The attached patch is identical to my first patch that fixed this bug 
> but failed sixtrack in SPEC with the exception of the last hunk.
> 
> The last hunk adds the test for end file to st_read where it belongs.
> 
> Regression tested on x86-64 and SPEC tested by Seongbae Park.
> 
> Also tested with Harald Anlauf's new test case, modified and attached as 
> well.
> 
> I want to especially thank Seongbae and Harald for their assistance 
> testing and finding the test cases.
> 
> I hope this settles it with this bug. (humbly)
> 
> OK for Trunk?
> 
> Regards,
> 
> Jerry
> 
> 2007-03-30  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
> 
>     PR libgfortran/31052
>     * io/file_position (st_rewind): Remove use of
>     test_endfile. Don't seek if already at 0 position.  Use new is_special
>     function to set endfile state.
>     * io/open.c (test_endfile): Delete this function.
>     (edit_modes): Delete call to test_endfile.
>     (new_unit): Likewise.
>     * io/io.h: Delete prototype for test_endfile.  Add prototype
>     for is_special.
>     * io/transfer.c (next_record_r): Remove use of test_endfile.
>     (st_read):  Add test for end file condition and adjust status.
> 
> 
> ------------------------------------------------------------------------
> 
> Index: io/open.c
> ===================================================================
> *** io/open.c	(revision 123355)
> --- io/open.c	(working copy)
> *************** static const st_option convert_opt[] =
> *** 109,127 ****
>     { NULL, 0}
>   };
>   
> - /* Given a unit, test to see if the file is positioned at the terminal
> -    point, and if so, change state from NO_ENDFILE flag to AT_ENDFILE.
> -    This prevents us from changing the state from AFTER_ENDFILE to
> -    AT_ENDFILE.  */
> - 
> - void
> - test_endfile (gfc_unit * u)
> - {
> -   if (u->endfile == NO_ENDFILE && file_length (u->s) == file_position (u->s))
> -     u->endfile = AT_ENDFILE;
> - }
> - 
> - 
>   /* Change the modes of a file, those that are allowed * to be
>      changed.  */
>   
> --- 109,114 ----
> *************** edit_modes (st_parameter_open *opp, gfc_
> *** 208,215 ****
>   
>         u->current_record = 0;
>         u->last_record = 0;
> - 
> -       test_endfile (u);		/* We might be at the end.  */
>         break;
>   
>       case POSITION_APPEND:
> --- 195,200 ----
> *************** new_unit (st_parameter_open *opp, gfc_un
> *** 487,499 ****
>     memmove (u->file, opp->file, opp->file_len);
>     u->file_len = opp->file_len;
>   
> -   /* Curiously, the standard requires that the
> -      position specifier be ignored for new files so a newly connected
> -      file starts out at the initial point.  We still need to figure
> -      out if the file is at the end or not.  */
> - 
> -   test_endfile (u);
> - 
>     if (flags->status == STATUS_SCRATCH && opp->file != NULL)
>       free_mem (opp->file);
>     return u;
> --- 472,477 ----
> Index: io/io.h
> ===================================================================
> *** io/io.h	(revision 123355)
> --- io/io.h	(working copy)
> *************** internal_proto(unlock_unit);
> *** 695,703 ****
>   
>   /* open.c */
>   
> - extern void test_endfile (gfc_unit *);
> - internal_proto(test_endfile);
> - 
>   extern gfc_unit *new_unit (st_parameter_open *, gfc_unit *, unit_flags *);
>   internal_proto(new_unit);
>   
> --- 695,700 ----
> Index: io/transfer.c
> ===================================================================
> *** io/transfer.c	(revision 123355)
> --- io/transfer.c	(working copy)
> *************** next_record_r (st_parameter_dt *dtp)
> *** 2227,2236 ****
>   
>         break;
>       }
> - 
> -   if (dtp->u.p.current_unit->flags.access == ACCESS_SEQUENTIAL
> -       && !dtp->u.p.namelist_mode)
> -     test_endfile (dtp->u.p.current_unit);
>   }
>   
>   
> --- 2227,2232 ----
> *************** st_read (st_parameter_dt *dtp)
> *** 2703,2708 ****
> --- 2699,2707 ----
>       switch (dtp->u.p.current_unit->endfile)
>         {
>         case NO_ENDFILE:
> + 	if (file_length (dtp->u.p.current_unit->s)
> + 	    == file_position (dtp->u.p.current_unit->s))
> + 	  dtp->u.p.current_unit->endfile = AT_ENDFILE;
>   	break;
>   
>         case AT_ENDFILE:
> 
> 
> ------------------------------------------------------------------------
> 
> ! { dg-do run }
> ! PR31052 Bad IOSTAT values when readings NAMELISTs past EOF.
> ! Patch derived from PR, submitted by Jerry DeLisle <jvdelisle@gcc.gnu.org>
> program gfcbug61
>   implicit none
>   integer, parameter :: nmlunit = 12    ! Namelist unit
>   integer            :: stat
> 
>   open (nmlunit, file="gfcbug61a.nml", status='old', action="READ")
> 
>   ! The call to position_nml is contained in the subroutine
>   rewind (nmlunit)
>   call read_report (nmlunit, stat)
>   rewind (nmlunit)
>   call position_nml (nmlunit, 'MISSING', stat)
>   rewind (nmlunit)
>   call read_report (nmlunit, stat)              ! gfortran fails here
> 
> contains
> 
>   subroutine position_nml (unit, name, status)
>     ! Check for presence of namelist 'name'
>     integer                      :: unit, status
>     character(len=*), intent(in) :: name
> 
>     character(len=255) :: line
>     integer            :: ios, idx
>     logical            :: first
> 
>     first = .true.
>     status = 0
>     do
>        line = ""
>        read (unit,'(a)',iostat=ios) line
>        if (ios < 0) then
>           ! EOF encountered!
>           backspace (unit)
>           status = -1
>           return
>        else if (ios > 0) then
>           ! Error encountered!
>           status = +1
>           return
>        end if
>        idx = index (line, "&"//trim (name))
>        if (idx > 0) then
>           backspace (unit)
>           return
>        end if
>     end do
>   end subroutine position_nml
> 
>   subroutine read_report (unit, status)
>     integer :: unit, status
> 
>     integer            :: iuse, ios
>     !------------------
>     ! Namelist 'REPORT'
>     !------------------
>     character(len=12) :: type
>     namelist /REPORT/ type
>     !-------------------------------------
>     ! Loop to read namelist multiple times
>     !-------------------------------------
>     iuse = 0
>     do
>        !----------------------------------------
>        ! Preset namelist variables with defaults
>        !----------------------------------------
>        type      = ''
>        !--------------
>        ! Read namelist
>        !--------------
>        call position_nml (unit, "REPORT", status)
>        if (stat /= 0) then
>           ios = status
> 	  if (iuse /= 2) call abort()
>           return
>        end if
>        read (unit, nml=REPORT, iostat=ios)
>        if (ios /= 0) exit
>        iuse = iuse + 1
>     end do
>     status = ios
>   end subroutine read_report
> 
> end program gfcbug61

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

Jerry DeLisle wrote:
:REVIEWMAIL:
> 
> Here is a final installment with test case.  I eliminated the 
> test_endfile function.  Then I added a new function is_special () that 
> returns true if the special file bit is set in the stream structure.
> 
The attached patch is identical to my first patch that fixed this bug but failed 
sixtrack in SPEC with the exception of the last hunk.

The last hunk adds the test for end file to st_read where it belongs.

Regression tested on x86-64 and SPEC tested by Seongbae Park.

Also tested with Harald Anlauf's new test case, modified and attached as well.

I want to especially thank Seongbae and Harald for their assistance testing and 
finding the test cases.

I hope this settles it with this bug. (humbly)

OK for Trunk?

Regards,

Jerry

2007-03-30  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

     PR libgfortran/31052
     * io/file_position (st_rewind): Remove use of
     test_endfile. Don't seek if already at 0 position.  Use new is_special
     function to set endfile state.
     * io/open.c (test_endfile): Delete this function.
     (edit_modes): Delete call to test_endfile.
     (new_unit): Likewise.
     * io/io.h: Delete prototype for test_endfile.  Add prototype
     for is_special.
     * io/transfer.c (next_record_r): Remove use of test_endfile.
     (st_read):  Add test for end file condition and adjust status.

[-- Attachment #2: sixtrack-2.diff --]
[-- Type: text/x-patch, Size: 2759 bytes --]

Index: io/open.c
===================================================================
*** io/open.c	(revision 123355)
--- io/open.c	(working copy)
*************** static const st_option convert_opt[] =
*** 109,127 ****
    { NULL, 0}
  };
  
- /* Given a unit, test to see if the file is positioned at the terminal
-    point, and if so, change state from NO_ENDFILE flag to AT_ENDFILE.
-    This prevents us from changing the state from AFTER_ENDFILE to
-    AT_ENDFILE.  */
- 
- void
- test_endfile (gfc_unit * u)
- {
-   if (u->endfile == NO_ENDFILE && file_length (u->s) == file_position (u->s))
-     u->endfile = AT_ENDFILE;
- }
- 
- 
  /* Change the modes of a file, those that are allowed * to be
     changed.  */
  
--- 109,114 ----
*************** edit_modes (st_parameter_open *opp, gfc_
*** 208,215 ****
  
        u->current_record = 0;
        u->last_record = 0;
- 
-       test_endfile (u);		/* We might be at the end.  */
        break;
  
      case POSITION_APPEND:
--- 195,200 ----
*************** new_unit (st_parameter_open *opp, gfc_un
*** 487,499 ****
    memmove (u->file, opp->file, opp->file_len);
    u->file_len = opp->file_len;
  
-   /* Curiously, the standard requires that the
-      position specifier be ignored for new files so a newly connected
-      file starts out at the initial point.  We still need to figure
-      out if the file is at the end or not.  */
- 
-   test_endfile (u);
- 
    if (flags->status == STATUS_SCRATCH && opp->file != NULL)
      free_mem (opp->file);
    return u;
--- 472,477 ----
Index: io/io.h
===================================================================
*** io/io.h	(revision 123355)
--- io/io.h	(working copy)
*************** internal_proto(unlock_unit);
*** 695,703 ****
  
  /* open.c */
  
- extern void test_endfile (gfc_unit *);
- internal_proto(test_endfile);
- 
  extern gfc_unit *new_unit (st_parameter_open *, gfc_unit *, unit_flags *);
  internal_proto(new_unit);
  
--- 695,700 ----
Index: io/transfer.c
===================================================================
*** io/transfer.c	(revision 123355)
--- io/transfer.c	(working copy)
*************** next_record_r (st_parameter_dt *dtp)
*** 2227,2236 ****
  
        break;
      }
- 
-   if (dtp->u.p.current_unit->flags.access == ACCESS_SEQUENTIAL
-       && !dtp->u.p.namelist_mode)
-     test_endfile (dtp->u.p.current_unit);
  }
  
  
--- 2227,2232 ----
*************** st_read (st_parameter_dt *dtp)
*** 2703,2708 ****
--- 2699,2707 ----
      switch (dtp->u.p.current_unit->endfile)
        {
        case NO_ENDFILE:
+ 	if (file_length (dtp->u.p.current_unit->s)
+ 	    == file_position (dtp->u.p.current_unit->s))
+ 	  dtp->u.p.current_unit->endfile = AT_ENDFILE;
  	break;
  
        case AT_ENDFILE:

[-- Attachment #3: namelist_28.f90 --]
[-- Type: text/x-fortran, Size: 2359 bytes --]

! { dg-do run }
! PR31052 Bad IOSTAT values when readings NAMELISTs past EOF.
! Patch derived from PR, submitted by Jerry DeLisle <jvdelisle@gcc.gnu.org>
program gfcbug61
  implicit none
  integer, parameter :: nmlunit = 12    ! Namelist unit
  integer            :: stat

  open (nmlunit, file="gfcbug61a.nml", status='old', action="READ")

  ! The call to position_nml is contained in the subroutine
  rewind (nmlunit)
  call read_report (nmlunit, stat)
  rewind (nmlunit)
  call position_nml (nmlunit, 'MISSING', stat)
  rewind (nmlunit)
  call read_report (nmlunit, stat)              ! gfortran fails here

contains

  subroutine position_nml (unit, name, status)
    ! Check for presence of namelist 'name'
    integer                      :: unit, status
    character(len=*), intent(in) :: name

    character(len=255) :: line
    integer            :: ios, idx
    logical            :: first

    first = .true.
    status = 0
    do
       line = ""
       read (unit,'(a)',iostat=ios) line
       if (ios < 0) then
          ! EOF encountered!
          backspace (unit)
          status = -1
          return
       else if (ios > 0) then
          ! Error encountered!
          status = +1
          return
       end if
       idx = index (line, "&"//trim (name))
       if (idx > 0) then
          backspace (unit)
          return
       end if
    end do
  end subroutine position_nml

  subroutine read_report (unit, status)
    integer :: unit, status

    integer            :: iuse, ios
    !------------------
    ! Namelist 'REPORT'
    !------------------
    character(len=12) :: type
    namelist /REPORT/ type
    !-------------------------------------
    ! Loop to read namelist multiple times
    !-------------------------------------
    iuse = 0
    do
       !----------------------------------------
       ! Preset namelist variables with defaults
       !----------------------------------------
       type      = ''
       !--------------
       ! Read namelist
       !--------------
       call position_nml (unit, "REPORT", status)
       if (stat /= 0) then
          ios = status
	  if (iuse /= 2) call abort()
          return
       end if
       read (unit, nml=REPORT, iostat=ios)
       if (ios /= 0) exit
       iuse = iuse + 1
    end do
    status = ios
  end subroutine read_report

end program gfcbug61

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[FX Coudert]

:REVIEWMAIL:

> 2007-03-13  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
>
> 	PR libgfortran/31052
> 	* io/file_position (st_rewind): Fix comments. Remove use of
> 	test_endfile. Don't seek if already at 0 position.  Use new  
> is_special
> 	function to set endfile state.
> 	* io/open.c (test_endfile): Delete this function.
> 	* io/io.h: Delete prototype for test_endfile.  Add prototype
> 	for is_special.
> 	* io/unix.c (is_special): New function. Fix whitespace.
> 	* io/transfer.c (next_record_r): Remove use of test_endfile.

This patch is OK if you can confirm that you reviewed and tested that  
it doesn't affect badly the behaviour for special files other than / 
dev/null.

On a more general note, looking at the code in libgfortran/io/unix.c,  
I think we're starting to use special_file to represent too many  
things (/dev/null, terminals, ...)  Maybe we should have a few  
separate attributes, like
   unsigned special_file:1;
   unsigned tty:1;
   unsigned seekable_special:1;

Anyway, you and Thomas K. are certainly our I/O experts here, so I'd  
appreciate your opinion on this ranting of mine, unrelatedly to you  
commiting the patch if or when you have it tested on different  
special files.

Thanks,
FX

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]

:ADDPATCH fortran:

Here is a final installment with test case.  I eliminated the test_endfile 
function.  Then I added a new function is_special () that returns true if the 
special file bit is set in the stream structure.

To retain compatibility with g77 behavior and previous versions of gfortran, I 
used this new function in st_rewind.  The reason this is necessary is because a 
read from /dev/null should always return EOF no matter what and if a user for 
some reason does a rewind, we won't get the EOF because the following rewind 
sets the NO_ENDFILE flag.

I have added some notes to identify where additional logic can be added for 
special files in the future with rewind.  We could, for example, generate a 
diagnostic message.

I also cleaned up some comments and whitespace here and there.

Regression tested on x86-64-Gnu/Linux.  New test case provided.

OK for trunk? and 4.2 after freeze and a little time for exercising on trunk?

Regards,

Jerry

2007-03-13  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/31052
	* io/file_position (st_rewind): Fix comments. Remove use of
	test_endfile. Don't seek if already at 0 position.  Use new is_special
	function to set endfile state.
	* io/open.c (test_endfile): Delete this function.
	* io/io.h: Delete prototype for test_endfile.  Add prototype
	for is_special.
	* io/unix.c (is_special): New function. Fix whitespace.
	* io/transfer.c (next_record_r): Remove use of test_endfile.


[-- Attachment #2: pr31052-c.diff --]
[-- Type: text/x-patch, Size: 6828 bytes --]

Index: file_pos.c
===================================================================
*** file_pos.c	(revision 122762)
--- file_pos.c	(working copy)
*************** st_rewind (st_parameter_filepos *fpp)
*** 297,320 ****
        else
  	{
  	  /* Flush the buffers.  If we have been writing to the file, the last
! 	       written record is the last record in the file, so truncate the
! 	       file now.  Reset to read mode so two consecutive rewind
! 	       statements do not delete the file contents.  */
  	  flush (u->s);
  	  if (u->mode == WRITING && u->flags.access != ACCESS_STREAM)
  	    struncate (u->s);
  
  	  u->mode = READING;
  	  u->last_record = 0;
! 	  if (sseek (u->s, 0) == FAILURE)
  	    generate_error (&fpp->common, ERROR_OS, NULL);
  
! 	  u->endfile = NO_ENDFILE;
  	  u->current_record = 0;
  	  u->bytes_left = 0;
  	  u->strm_pos = 1;
  	  u->read_bad = 0;
- 	  test_endfile (u);
  	}
        /* Update position for INQUIRE.  */
        u->flags.position = POSITION_REWIND;
--- 297,332 ----
        else
  	{
  	  /* Flush the buffers.  If we have been writing to the file, the last
! 	     written record is the last record in the file, so truncate the
! 	     file now.  Reset to read mode so two consecutive rewind
! 	     statements do not delete the file contents.  */
  	  flush (u->s);
  	  if (u->mode == WRITING && u->flags.access != ACCESS_STREAM)
  	    struncate (u->s);
  
  	  u->mode = READING;
  	  u->last_record = 0;
! 	  if (file_position (u->s) != 0 && sseek (u->s, 0) == FAILURE)
  	    generate_error (&fpp->common, ERROR_OS, NULL);
  
! 	  /* Handle special files like /dev/null differently.  */
! 	  if (!is_special (u->s))
! 	    {
! 	      /* We are rewinding so we are not at the end.  */
! 	      u->endfile = NO_ENDFILE;
! 	    }
! 	  else
! 	    {
! 	      /* Set this for compatibilty with g77 for /dev/null.  */
! 	      if (file_length (u->s) == 0  && file_position (u->s) == 0)
! 		u->endfile = AT_ENDFILE;
! 	      /* Future refinements on special files can go here.  */
! 	    }
! 
  	  u->current_record = 0;
  	  u->bytes_left = 0;
  	  u->strm_pos = 1;
  	  u->read_bad = 0;
  	}
        /* Update position for INQUIRE.  */
        u->flags.position = POSITION_REWIND;
Index: open.c
===================================================================
*** open.c	(revision 122763)
--- open.c	(working copy)
*************** static const st_option convert_opt[] =
*** 109,127 ****
    { NULL, 0}
  };
  
- /* Given a unit, test to see if the file is positioned at the terminal
-    point, and if so, change state from NO_ENDFILE flag to AT_ENDFILE.
-    This prevents us from changing the state from AFTER_ENDFILE to
-    AT_ENDFILE.  */
- 
- void
- test_endfile (gfc_unit * u)
- {
-   if (u->endfile == NO_ENDFILE && file_length (u->s) == file_position (u->s))
-     u->endfile = AT_ENDFILE;
- }
- 
- 
  /* Change the modes of a file, those that are allowed * to be
     changed.  */
  
--- 109,114 ----
*************** edit_modes (st_parameter_open *opp, gfc_
*** 208,215 ****
  
        u->current_record = 0;
        u->last_record = 0;
- 
-       test_endfile (u);		/* We might be at the end.  */
        break;
  
      case POSITION_APPEND:
--- 195,200 ----
*************** new_unit (st_parameter_open *opp, gfc_un
*** 486,498 ****
    memmove (u->file, opp->file, opp->file_len);
    u->file_len = opp->file_len;
  
-   /* Curiously, the standard requires that the
-      position specifier be ignored for new files so a newly connected
-      file starts out that the initial point.  We still need to figure
-      out if the file is at the end or not.  */
- 
-   test_endfile (u);
- 
    if (flags->status == STATUS_SCRATCH && opp->file != NULL)
      free_mem (opp->file);
    return u;
--- 471,476 ----
Index: io.h
===================================================================
*** io.h	(revision 122762)
--- io.h	(working copy)
*************** internal_proto(file_position);
*** 622,627 ****
--- 622,630 ----
  extern int is_seekable (stream *);
  internal_proto(is_seekable);
  
+ extern int is_special (stream *);
+ internal_proto(is_special);
+ 
  extern int is_preconnected (stream *);
  internal_proto(is_preconnected);
  
*************** internal_proto(unlock_unit);
*** 691,699 ****
  
  /* open.c */
  
- extern void test_endfile (gfc_unit *);
- internal_proto(test_endfile);
- 
  extern gfc_unit *new_unit (st_parameter_open *, gfc_unit *, unit_flags *);
  internal_proto(new_unit);
  
--- 694,699 ----
Index: unix.c
===================================================================
*** unix.c	(revision 122762)
--- unix.c	(working copy)
*************** file_length (stream * s)
*** 1872,1878 ****
  /* file_position()-- Return the current position of the file */
  
  gfc_offset
! file_position (stream * s)
  {
    return ((unix_stream *) s)->logical_offset;
  }
--- 1872,1878 ----
  /* file_position()-- Return the current position of the file */
  
  gfc_offset
! file_position (stream *s)
  {
    return ((unix_stream *) s)->logical_offset;
  }
*************** file_position (stream * s)
*** 1882,1894 ****
   * it is not */
  
  int
! is_seekable (stream * s)
  {
    /* By convention, if file_length == -1, the file is not
       seekable.  */
    return ((unix_stream *) s)->file_length!=-1;
  }
  
  try
  flush (stream *s)
  {
--- 1882,1903 ----
   * it is not */
  
  int
! is_seekable (stream *s)
  {
    /* By convention, if file_length == -1, the file is not
       seekable.  */
    return ((unix_stream *) s)->file_length!=-1;
  }
  
+ 
+ /* is_special()-- Return nonzero if the stream is not a regular file.  */
+ 
+ is_special (stream *s)
+ {
+   return ((unix_stream *) s)->special_file;
+ }
+ 
+ 
  try
  flush (stream *s)
  {
Index: transfer.c
===================================================================
*** transfer.c	(revision 122763)
--- transfer.c	(working copy)
*************** next_record_r (st_parameter_dt *dtp)
*** 2217,2225 ****
  
        break;
      }
- 
-   if (dtp->u.p.current_unit->flags.access == ACCESS_SEQUENTIAL)
-     test_endfile (dtp->u.p.current_unit);
  }
  
  
--- 2217,2222 ----
*************** st_read (st_parameter_dt *dtp)
*** 2679,2688 ****
  
    data_transfer_init (dtp, 1);
  
!   /* Handle complications dealing with the endfile record.  It is
!      significant that this is the only place where ERROR_END is
!      generated.  Reading an end of file elsewhere is either end of
!      record or an I/O error. */
  
    if (dtp->u.p.current_unit->flags.access == ACCESS_SEQUENTIAL)
      switch (dtp->u.p.current_unit->endfile)
--- 2676,2682 ----
  
    data_transfer_init (dtp, 1);
  
!   /* Handle complications dealing with the endfile record. */
  
    if (dtp->u.p.current_unit->flags.access == ACCESS_SEQUENTIAL)
      switch (dtp->u.p.current_unit->endfile)

[-- Attachment #3: namelist_27.f90 --]
[-- Type: text/x-fortran, Size: 2831 bytes --]

! { dg-do run }
! PR31052 Bad IOSTAT values when readings NAMELISTs past EOF.
! Patch derived from PR, submitted by Jerry DeLisle <jvdelisle@gcc.gnu.org>
program gfcbug61
  implicit none
  integer            :: stat

  open (12, status="scratch")
  write (12, '(a)')"!================"
  write (12, '(a)')"! Namelist REPORT"
  write (12, '(a)')"!================"
  write (12, '(a)')" &REPORT type     = 'SYNOP' "
  write (12, '(a)')"         use      = 'active'"
  write (12, '(a)')"         max_proc = 20"
  write (12, '(a)')" /"
  write (12, '(a)')"! Other namelists..."
  write (12, '(a)')" &OTHER  i = 1 /"
  rewind (12)

  ! Read /REPORT/ the first time
  rewind (12)
  call position_nml (12, "REPORT", stat)
  if (stat.ne.0) call abort()
  if (stat == 0)  call read_report (12, stat)

  ! Comment out the following lines to hide the bug
  rewind (12)
  call position_nml (12, "MISSING", stat)
  if (stat.ne.-1)  call abort ()

  ! Read /REPORT/ again
  rewind (12)
  call position_nml (12, "REPORT", stat)
  if (stat.ne.0)  call abort()

contains

  subroutine position_nml (unit, name, status)
    ! Check for presence of namelist 'name'
    integer                      :: unit, status
    character(len=*), intent(in) :: name

    character(len=255) :: line
    integer            :: ios, idx
    logical            :: first

    first = .true.
    status = 0
    ios = 0
    line = ""
    do
       read (unit,'(a)',iostat=ios) line
       if (first) then
          first = .false.
       end if
       if (ios < 0) then
          ! EOF encountered!
          backspace (unit)
          status = -1
          return
       else if (ios > 0) then
          ! Error encountered!
          status = +1
          return
       end if
       idx = index (line, "&"//trim (name))
       if (idx > 0) then
          backspace (unit)
          return
       end if
    end do
  end subroutine position_nml

  subroutine read_report (unit, status)
    integer :: unit, status

    integer            :: iuse, ios
    !------------------
    ! Namelist 'REPORT'
    !------------------
    character(len=12) :: type, use
    integer           :: max_proc
    namelist /REPORT/ type, use, max_proc
    !-------------------------------------
    ! Loop to read namelist multiple times
    !-------------------------------------
    iuse = 0
    do
       !----------------------------------------
       ! Preset namelist variables with defaults
       !----------------------------------------
       type      = ''
       use       = ''
       max_proc  = -1
       !--------------
       ! Read namelist
       !--------------
       read (unit, nml=REPORT, iostat=ios)
       if (ios /= 0) exit
       iuse = iuse + 1
    end do
    if (iuse.ne.1) call abort()
    status = ios
  end subroutine read_report

end program gfcbug61

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

Steve Kargl wrote:
> On Sat, Mar 10, 2007 at 08:57:19AM -0800, Jerry DeLisle wrote:
>> Steve Kargl wrote:
>>> On Fri, Mar 09, 2007 at 11:45:33PM -0800, Jerry DeLisle wrote:
>>>> I would rather keep the patch as I have it and leave dev_null.f90 alone.  
>>>> In this case we are user proofing.  st_rewind is simply seeking to 
>>>> position zero of the file and then resetting all the internal libgfortran 
>>>> accounting stuff. /dev/null is a "special seekable" file.
>>>>
>>>> I have also tested this on an empty file and it works OK for that.  The 
>>>> adjustment in the patch occurs well after all the rewind stuff is done.
>>>>
>>>> I will await approval.
>>>>
>>> I have you tested with /dev/zero and /dev/random?
>>>
>> I had not, but now I have and the test_endfile does not "work" for a rewind 
>> on these.  In other words, I get an EOF condition on a read following a 
>> rewind.  It seems that the whole concept is askew.  The /dev/null is 
>> defined to always give an EOF on a read.  The others should never give an 
>> EOF.
>>
>> I want to give this some thinking time.  The OS should give the correct 
>> response and rewind should ideally have no effect.  I think I should 
>> address why it does have an effect now, with or without test_endfile.
>>
>> In the interim, I would like to fix the original PR by eliminating 
>> test_endfile and open a new PR related to the behavior of these special 
>> devices.
>>
>> Is there agreement on this?
>>
> 
> Yes.
> 
> The handling of special files (and fifo and pipes) is probably
> a messy system dependent pit.

Well, I completely removed the test_endfile function found in open.c and all of 
its invocations in several places.  I removed the prototype from io.h.

Regression tested, NIST tested, many io tested with some programs I have 
collected.  All pass with flying colors.  I suspect we worked our way out of 
needing it with other patches until it was redundant and no longer needed.  The 
only place it was needed is in st_rewind to handle that one odd case for dev/null.

Removing the call to rewind in the test case dev_null.f90 and that test case 
passes fine with the expected behavior, EOF on a read.

I will go ahead and work up the test case for PR31052 and resubmit a final patch.

Regards,

Jerry

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Steve Kargl]

On Sat, Mar 10, 2007 at 08:57:19AM -0800, Jerry DeLisle wrote:
> Steve Kargl wrote:
> >On Fri, Mar 09, 2007 at 11:45:33PM -0800, Jerry DeLisle wrote:
> >>I would rather keep the patch as I have it and leave dev_null.f90 alone.  
> >>In this case we are user proofing.  st_rewind is simply seeking to 
> >>position zero of the file and then resetting all the internal libgfortran 
> >>accounting stuff. /dev/null is a "special seekable" file.
> >>
> >>I have also tested this on an empty file and it works OK for that.  The 
> >>adjustment in the patch occurs well after all the rewind stuff is done.
> >>
> >>I will await approval.
> >>
> >
> >I have you tested with /dev/zero and /dev/random?
> >
> I had not, but now I have and the test_endfile does not "work" for a rewind 
> on these.  In other words, I get an EOF condition on a read following a 
> rewind.  It seems that the whole concept is askew.  The /dev/null is 
> defined to always give an EOF on a read.  The others should never give an 
> EOF.
> 
> I want to give this some thinking time.  The OS should give the correct 
> response and rewind should ideally have no effect.  I think I should 
> address why it does have an effect now, with or without test_endfile.
> 
> In the interim, I would like to fix the original PR by eliminating 
> test_endfile and open a new PR related to the behavior of these special 
> devices.
> 
> Is there agreement on this?
> 

Yes.

The handling of special files (and fifo and pipes) is probably
a messy system dependent pit.
-- 
Steve

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

Steve Kargl wrote:
> On Fri, Mar 09, 2007 at 11:45:33PM -0800, Jerry DeLisle wrote:
>> I would rather keep the patch as I have it and leave dev_null.f90 alone.  
>> In this case we are user proofing.  st_rewind is simply seeking to position 
>> zero of the file and then resetting all the internal libgfortran accounting 
>> stuff. /dev/null is a "special seekable" file.
>>
>> I have also tested this on an empty file and it works OK for that.  The 
>> adjustment in the patch occurs well after all the rewind stuff is done.
>>
>> I will await approval.
>>
> 
> I have you tested with /dev/zero and /dev/random?
> 
I had not, but now I have and the test_endfile does not "work" for a rewind on 
these.  In other words, I get an EOF condition on a read following a rewind.  It 
seems that the whole concept is askew.  The /dev/null is defined to always give 
an EOF on a read.  The others should never give an EOF.

I want to give this some thinking time.  The OS should give the correct response 
and rewind should ideally have no effect.  I think I should address why it does 
have an effect now, with or without test_endfile.

In the interim, I would like to fix the original PR by eliminating test_endfile 
and open a new PR related to the behavior of these special devices.

Is there agreement on this?

Jerry

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Steve Kargl]

On Fri, Mar 09, 2007 at 11:45:33PM -0800, Jerry DeLisle wrote:
>
> I would rather keep the patch as I have it and leave dev_null.f90 alone.  
> In this case we are user proofing.  st_rewind is simply seeking to position 
> zero of the file and then resetting all the internal libgfortran accounting 
> stuff. /dev/null is a "special seekable" file.
> 
> I have also tested this on an empty file and it works OK for that.  The 
> adjustment in the patch occurs well after all the rewind stuff is done.
> 
> I will await approval.
> 

I have you tested with /dev/zero and /dev/random?

-- 
Steve

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

Brooks Moses wrote:
> Jerry DeLisle wrote:
>> Steve Kargl wrote:
>>> I haven't read the PR nor your patch.  But, if
>>> someone is trying to rewind /dev/null, he/she
>>> is an idiot.  If it was me, I'd close the PR
>>> with a rather biting comment about stupidity.
>>
>> Easy guy, its the existing dev_null.f90 in gfortran.dg thats doing 
>> this rewind.   If you suggest we get rid of the rewind in there, then 
>> patching this PR is simply delete the call to test_endfile in st_rewind.
> 
> I suspect /dev/null there is acting as a proxy for other sources that 
> also can't be rewound, but can't be easily tested in a testcase.
> 
> I've certainly written codes that did rewinds which weren't obviously 
> necessary from the end-user standpoint, and I could certainly imagine an 
> unknowing user (or a forgetful user, if the user was me) hooking them up 
> to some source -- a named pipe, say -- which didn't support rewinding. 
> I'd be a bit annoyed if GFortran didn't handle that with a reasonable 
> error/IOSTAT result.
> 
> Thus, if you remove this from dev_null.f90, I'd want to recommend 
> checking to make sure that you're _only_ breaking /dev/null and not 
> breaking other non-rewindable sources, and ideally make sure that this 
> is still tested some other way in the testsuite.
> 
> - Brooks
> 
> 
I would rather keep the patch as I have it and leave dev_null.f90 alone.  In 
this case we are user proofing.  st_rewind is simply seeking to position zero of 
the file and then resetting all the internal libgfortran accounting stuff. 
/dev/null is a "special seekable" file.

I have also tested this on an empty file and it works OK for that.  The 
adjustment in the patch occurs well after all the rewind stuff is done.

I will await approval.

Thanks,

Jerry

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Brooks Moses]

Jerry DeLisle wrote:
> Steve Kargl wrote:
>> I haven't read the PR nor your patch.  But, if
>> someone is trying to rewind /dev/null, he/she
>> is an idiot.  If it was me, I'd close the PR
>> with a rather biting comment about stupidity.
>
> Easy guy, its the existing dev_null.f90 in gfortran.dg thats doing this rewind. 
>   If you suggest we get rid of the rewind in there, then patching this PR is 
> simply delete the call to test_endfile in st_rewind.

I suspect /dev/null there is acting as a proxy for other sources that 
also can't be rewound, but can't be easily tested in a testcase.

I've certainly written codes that did rewinds which weren't obviously 
necessary from the end-user standpoint, and I could certainly imagine an 
unknowing user (or a forgetful user, if the user was me) hooking them up 
to some source -- a named pipe, say -- which didn't support rewinding. 
I'd be a bit annoyed if GFortran didn't handle that with a reasonable 
error/IOSTAT result.

Thus, if you remove this from dev_null.f90, I'd want to recommend 
checking to make sure that you're _only_ breaking /dev/null and not 
breaking other non-rewindable sources, and ideally make sure that this 
is still tested some other way in the testsuite.

- Brooks

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

Steve Kargl wrote:
> On Fri, Mar 09, 2007 at 08:43:01PM -0800, Jerry DeLisle wrote:
>> This problem was not quite what I originally thought.  The function 
>> test_endfile was setting the endfile flag to AT_ENDFILE for the test case 
>> provided in the PR.
>>
>> Commenting out test_endfile in st_rewind resolves the test case but 
>> regresses dev_null.f90.  All other test cases past.  So, it appears that 
>> test_endfile is needed for the special case when rewinding /dev/null.  This 
>> seems weird to me to begin with, but how does one determine that a file is 
>> /dev/null?
>>
> 
> I haven't read the PR nor your patch.  But, if
> someone is trying to rewind /dev/null, he/she
> is an idiot.  If it was me, I'd close the PR
> with a rather biting comment about stupidity.
> 
Easy guy, its the existing dev_null.f90 in gfortran.dg thats doing this rewind. 
  If you suggest we get rid of the rewind in there, then patching this PR is 
simply delete the call to test_endfile in st_rewind.

-- jerry

Re: [patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Steve Kargl]

On Fri, Mar 09, 2007 at 08:43:01PM -0800, Jerry DeLisle wrote:
> 
> This problem was not quite what I originally thought.  The function 
> test_endfile was setting the endfile flag to AT_ENDFILE for the test case 
> provided in the PR.
> 
> Commenting out test_endfile in st_rewind resolves the test case but 
> regresses dev_null.f90.  All other test cases past.  So, it appears that 
> test_endfile is needed for the special case when rewinding /dev/null.  This 
> seems weird to me to begin with, but how does one determine that a file is 
> /dev/null?
> 

I haven't read the PR nor your patch.  But, if
someone is trying to rewind /dev/null, he/she
is an idiot.  If it was me, I'd close the PR
with a rather biting comment about stupidity.

-- 
Steve

[patch, libgfortran] PR31052 Bad IOSTAT values when readings NAMELISTs past EOF

[Jerry DeLisle]

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

:addpatch fortran:

Hi,

This problem was not quite what I originally thought.  The function test_endfile 
was setting the endfile flag to AT_ENDFILE for the test case provided in the PR.

Commenting out test_endfile in st_rewind resolves the test case but regresses 
dev_null.f90.  All other test cases past.  So, it appears that test_endfile is 
needed for the special case when rewinding /dev/null.  This seems weird to me to 
begin with, but how does one determine that a file is /dev/null?

Regardless, the attached patch resolves this issue by adding the additional 
condition that the file_length is zero before setting end_file to AT_ENDFILE.

Fully regression tested on x86-64-Gnu/Linux.

On the test case, I will convert the case in the PR to an equivalent testsuite 
version.

OK for trunk and then 4.2 after the branch freezes and reopens later?

Regards,

Jerry

2007-03-09  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/31052
	* io/open.c (test_endfile): Add condition of file_length equal to zero
	to prevent incorrect setting of endfile flag to AT_ENDFILE.
	(new_unit): Fix typo in comment.

[-- Attachment #2: pr32052.diff --]
[-- Type: text/x-patch, Size: 1216 bytes --]

Index: open.c
===================================================================
*** open.c	(revision 122763)
--- open.c	(working copy)
*************** static const st_option convert_opt[] =
*** 117,123 ****
  void
  test_endfile (gfc_unit * u)
  {
!   if (u->endfile == NO_ENDFILE && file_length (u->s) == file_position (u->s))
      u->endfile = AT_ENDFILE;
  }
  
--- 117,125 ----
  void
  test_endfile (gfc_unit * u)
  {
!   if (u->endfile == NO_ENDFILE
!       && file_length (u->s) == file_position (u->s)
!       && file_length (u->s) == 0)
      u->endfile = AT_ENDFILE;
  }
  
*************** new_unit (st_parameter_open *opp, gfc_un
*** 488,494 ****
  
    /* Curiously, the standard requires that the
       position specifier be ignored for new files so a newly connected
!      file starts out that the initial point.  We still need to figure
       out if the file is at the end or not.  */
  
    test_endfile (u);
--- 490,496 ----
  
    /* Curiously, the standard requires that the
       position specifier be ignored for new files so a newly connected
!      file starts out at the initial point.  We still need to figure
       out if the file is at the end or not.  */
  
    test_endfile (u);