diff mbox

[Fortran] PR57633 - Fix EOL handling with \r in list-directed I/O

Message ID 51C2C75F.70707@net-b.de
State New
Headers show

Commit Message

Tobias Burnus June 20, 2013, 9:11 a.m. UTC 
gfortran failed to correctly read the file
    line1,1,
    line2
with DOS (\r\n) line endings. As the code already set EOL for "\r", 
finish_list_read didn't call eat_line. Result: The attempt to read 
"line2" actually accessed the last byte of line one, namely "\n", which 
it regarded as zero-sized string.

That's fixed by the patch to next_char.

eat_separator is a separate issue and unrelated to the PR. I also failed 
to create a test case for it. In any case, I regard the following as wrong:

      case '\r':
       dtp->u.p.at_eol = 1;
...
        if (n != '\n')
         {
           unget_char (dtp, n);
           break;

As the code explicitly does not regard "\r" as EOL in this case, I 
believe EOL shouldn't be set here.

(Recall, Unix (MacOS X, Linux, ...) have '\n' while DOS/Windows has 
"\r\n". While '\r' as line break exists (old Macs, pre MacOS X), 
gfortran does not support formatted I/O with "\r" record markers.)


Build and regtested on x86-64-gnu-linux.
OK for the trunk?

Tobias

Comments

Janne Blomqvist June 20, 2013, 10:34 a.m. UTC | #1 
On Thu, Jun 20, 2013 at 12:11 PM, Tobias Burnus <burnus@net-b.de> wrote:
> gfortran failed to correctly read the file
>    line1,1,
>    line2
> with DOS (\r\n) line endings. As the code already set EOL for "\r",
> finish_list_read didn't call eat_line. Result: The attempt to read "line2"
> actually accessed the last byte of line one, namely "\n", which it regarded
> as zero-sized string.
>
> That's fixed by the patch to next_char.
>
> eat_separator is a separate issue and unrelated to the PR. I also failed to
> create a test case for it. In any case, I regard the following as wrong:
>
>      case '\r':
>       dtp->u.p.at_eol = 1;
> ...
>        if (n != '\n')
>         {
>           unget_char (dtp, n);
>           break;
>
> As the code explicitly does not regard "\r" as EOL in this case, I believe
> EOL shouldn't be set here.
>
> (Recall, Unix (MacOS X, Linux, ...) have '\n' while DOS/Windows has "\r\n".
> While '\r' as line break exists (old Macs, pre MacOS X), gfortran does not
> support formatted I/O with "\r" record markers.)
>
>
> Build and regtested on x86-64-gnu-linux.
> OK for the trunk?
>
> Tobias

Ok, thanks.


--
Janne Blomqvist
diff mbox

Patch

2013-06-20  Tobias Burnus  <burnus@net-b.de>

	PR fortran/57633
	* io/list_read.c (next_char, eat_separator): Don't set EOL for \r.

2013-06-20  Tobias Burnus  <burnus@net-b.de>

	PR fortran/57633
	* gfortran.dg/list_read_11.f90: New.

diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index c8a1bdfc..82a98a5 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -235,21 +235,21 @@  next_char (st_parameter_dt *dtp)
 	    }
 	}
     }
   else
     {
       c = fbuf_getc (dtp->u.p.current_unit);
       if (c != EOF && is_stream_io (dtp))
 	dtp->u.p.current_unit->strm_pos++;
     }
 done:
-  dtp->u.p.at_eol = (c == '\n' || c == '\r' || c == EOF);
+  dtp->u.p.at_eol = (c == '\n' || c == EOF);
   return c;
 }
 
 
 /* Push a character back onto the input.  */
 
 static void
 unget_char (st_parameter_dt *dtp, int c)
 {
   dtp->u.p.last_char = c;
@@ -327,21 +327,20 @@  eat_separator (st_parameter_dt *dtp)
     case ';':
       dtp->u.p.comma_flag = 1;
       eat_spaces (dtp);
       break;
 
     case '/':
       dtp->u.p.input_complete = 1;
       break;
 
     case '\r':
-      dtp->u.p.at_eol = 1;
       if ((n = next_char(dtp)) == EOF)
 	return LIBERROR_END;
       if (n != '\n')
 	{
 	  unget_char (dtp, n);
 	  break;
 	}
     /* Fall through.  */
     case '\n':
       dtp->u.p.at_eol = 1;
--- /dev/null	2013-06-20 10:08:42.876937873 +0200
+++ gcc/gcc/testsuite/gfortran.dg/list_read_11.f90	2013-06-20 10:55:13.329549458 +0200
@@ -0,0 +1,38 @@ 
+! { dg-do run }
+! { dg-options "-fbackslash" }
+!
+! PR fortran/57633
+!
+program teststuff
+  implicit none
+  integer::a
+  character(len=10)::s1,s2
+
+  open(11,file="testcase.txt",form='unformatted',access='stream',status='new')
+  write(11) 'line1,1,\r\nline2'
+  close(11)
+
+  open(11,file="testcase.txt",form='formatted')
+  s1 = repeat('x', len(s1))
+  a = 99
+  read(11,*)s1,a
+  if (s1 /= "line1" .or. a /= 1) call abort()
+
+  s1 = repeat('x', len(s1))
+  read(11,"(a)")s1
+  close(11,status="delete")
+  if (s1 /= "line2") call abort()
+
+
+  open(11,file="testcase.txt",form='unformatted',access='stream',status='new')
+  write(11) 'word1\rword2,\n'
+  close(11)
+
+  open(11,file="testcase.txt",form='formatted')
+  s1 = repeat('x', len(s1))
+  s2 = repeat('x', len(s1))
+  read(11,*)s1,s2
+  close(11,status="delete")
+  if (s1 /= "word1") call abort()
+  if (s2 /= "word2") call abort()
+end program teststuff