Merge: Filereader fix
authorJean Privat <jean@pryen.org>
Tue, 5 May 2015 01:09:50 +0000 (21:09 -0400)
committerJean Privat <jean@pryen.org>
Tue, 5 May 2015 01:09:50 +0000 (21:09 -0400)
Since 39fcf4a75aaec646f0b60d404dfcc5cb63dcb473 and because people prefer the Python/Ruby semantic to the C semantic, eof is blocking.

This PR rewrite `BufferedReader.read` to follow the Python/Ruby semantic: if there is 1 character in the system buffer, and that the programmer asks for 2, the program will wait to have 1 more character, that can block the whole program if we are waiting the missing character from some keyboard, pipe or tcp connection.

Here a comparaison of the various specifications:

The protocol is the following: I `read(4)` bytes from stdin and print them. On stdin, I write `ab\n` (3 bytes) then `cd\n` (3 other bytes) and see when the the read does Its job.
In the following, I only give the outputs since the input will be the same and adding it will require to distinguish them with the output.

~~~sh
$ ./nit_old -e 'print sys.stdin.read(4)'
a
~~~

WTF? Why only char `a` was printed? OK the implementation before the PR was more buggy than expected.

~~~sh
$ ./nit_new -e 'print sys.stdin.read(4)'
ab
c
~~~

As I expected, nothing is printed after the first `ab\n` because exactly 4 bytes are waited for, then printed. Thus we get `ab\nc`. The extra `d\n` are lost in the buffer of Nit.

~~~sh
$ ruby -e 'puts $stdin.read(4)'
ab
c
$ d
bash: d : commande introuvable
~~~

Same behavior with Ruby, so nothing is printed after the first `ab\n`.
Nice surprise however, the extra `d\n` are not lost but kept in system buffer, so still available (and read) when the shell take back the control. I am not really sure which behavior I prefer. The Ruby one might be saner from an OS point of view; but since I was surprised, one can assume that the POLA level is not that high.

~~~sh
$ python <(echo 'import sys;print(sys.stdin.read(4))')
ab
c
~~~

So exactly the same behavior than Nit with this PR.

Close #1264

Pull-Request: #1297
Reviewed-by: Jean Privat <jean@pryen.org>
Reviewed-by: Alexandre Terrasa <alexandre@moz-code.org>

lib/standard/stream.nit
tests/sav/test_bufferedfilereader.res [new file with mode: 0644]
tests/test_bufferedfilereader.nit [new file with mode: 0644]

index 7df1bd1..e0d3a67 100644 (file)
@@ -395,20 +395,18 @@ abstract class BufferedReader
        redef fun read(i)
        do
                if last_error != null then return ""
-               if _buffer.length == _buffer_pos then
-                       if not eof then
-                               return read(i)
-                       end
-                       return ""
-               end
-               if _buffer_pos + i >= _buffer.length then
-                       var from = _buffer_pos
-                       _buffer_pos = _buffer.length
-                       if from == 0 then return _buffer.to_s
-                       return _buffer.substring_from(from).to_s
+               if eof then return ""
+               var p = _buffer_pos
+               var bufsp = _buffer.length - p
+               if bufsp >= i then
+                       _buffer_pos += i
+                       return _buffer.substring(p, i).to_s
                end
-               _buffer_pos += i
-               return _buffer.substring(_buffer_pos - i, i).to_s
+               _buffer_pos = _buffer.length
+               var readln = _buffer.length - p
+               var s = _buffer.substring(p, readln).to_s
+               fill_buffer
+               return s + read(i - readln)
        end
 
        redef fun read_all
diff --git a/tests/sav/test_bufferedfilereader.res b/tests/sav/test_bufferedfilereader.res
new file mode 100644 (file)
index 0000000..3c68ca8
--- /dev/null
@@ -0,0 +1,244 @@
+Read 4 chars: # Th
+Read 4 chars: is f
+Read 4 chars: ile 
+Read 4 chars: is p
+Read 4 chars: art 
+Read 4 chars: of N
+Read 4 chars: IT (
+Read 4 chars:  htt
+Read 4 chars: p://
+Read 4 chars: www.
+Read 4 chars: nitl
+Read 4 chars: angu
+Read 4 chars: age.
+Read 4 chars: org 
+Read 4 chars: ).
+#
+Read 4 chars: 
+# C
+Read 4 chars: opyr
+Read 4 chars: ight
+Read 4 chars:  201
+Read 4 chars: 3 Al
+Read 4 chars: exis
+Read 4 chars:  Laf
+Read 4 chars: erri
+Read 4 chars: ère
+Read 4 chars:  <al
+Read 4 chars: exis
+Read 4 chars: .laf
+Read 4 chars: @xym
+Read 4 chars: us.n
+Read 4 chars: et>
+
+Read 4 chars: #
+# 
+Read 4 chars: Lice
+Read 4 chars: nsed
+Read 4 chars:  und
+Read 4 chars: er t
+Read 4 chars: he A
+Read 4 chars: pach
+Read 4 chars: e Li
+Read 4 chars: cens
+Read 4 chars: e, V
+Read 4 chars: ersi
+Read 4 chars: on 2
+Read 4 chars: .0 (
+Read 4 chars: the 
+Read 4 chars: "Lic
+Read 4 chars: ense
+Read 4 chars: ");
+
+Read 4 chars: # yo
+Read 4 chars: u ma
+Read 4 chars: y no
+Read 4 chars: t us
+Read 4 chars: e th
+Read 4 chars: is f
+Read 4 chars: ile 
+Read 4 chars: exce
+Read 4 chars: pt i
+Read 4 chars: n co
+Read 4 chars: mpli
+Read 4 chars: ance
+Read 4 chars:  wit
+Read 4 chars: h th
+Read 4 chars: e Li
+Read 4 chars: cens
+Read 4 chars: e.
+#
+Read 4 chars:  You
+Read 4 chars:  may
+Read 4 chars:  obt
+Read 4 chars: ain 
+Read 4 chars: a co
+Read 4 chars: py o
+Read 4 chars: f th
+Read 4 chars: e Li
+Read 4 chars: cens
+Read 4 chars: e at
+Read 4 chars: 
+#
+#
+Read 4 chars:     
+Read 4 chars:  htt
+Read 4 chars: p://
+Read 4 chars: www.
+Read 4 chars: apac
+Read 4 chars: he.o
+Read 4 chars: rg/l
+Read 4 chars: icen
+Read 4 chars: ses/
+Read 4 chars: LICE
+Read 4 chars: NSE-
+Read 4 chars: 2.0
+
+Read 4 chars: #
+# 
+Read 4 chars: Unle
+Read 4 chars: ss r
+Read 4 chars: equi
+Read 4 chars: red 
+Read 4 chars: by a
+Read 4 chars: ppli
+Read 4 chars: cabl
+Read 4 chars: e la
+Read 4 chars: w or
+Read 4 chars:  agr
+Read 4 chars: eed 
+Read 4 chars: to i
+Read 4 chars: n wr
+Read 4 chars: itin
+Read 4 chars: g, s
+Read 4 chars: oftw
+Read 4 chars: are
+
+Read 4 chars: # di
+Read 4 chars: stri
+Read 4 chars: bute
+Read 4 chars: d un
+Read 4 chars: der 
+Read 4 chars: the 
+Read 4 chars: Lice
+Read 4 chars: nse 
+Read 4 chars: is d
+Read 4 chars: istr
+Read 4 chars: ibut
+Read 4 chars: ed o
+Read 4 chars: n an
+Read 4 chars:  "AS
+Read 4 chars:  IS"
+Read 4 chars:  BAS
+Read 4 chars: IS,
+
+Read 4 chars: # WI
+Read 4 chars: THOU
+Read 4 chars: T WA
+Read 4 chars: RRAN
+Read 4 chars: TIES
+Read 4 chars:  OR 
+Read 4 chars: COND
+Read 4 chars: ITIO
+Read 4 chars: NS O
+Read 4 chars: F AN
+Read 4 chars: Y KI
+Read 4 chars: ND, 
+Read 4 chars: eith
+Read 4 chars: er e
+Read 4 chars: xpre
+Read 4 chars: ss o
+Read 4 chars: r im
+Read 4 chars: plie
+Read 4 chars: d.
+#
+Read 4 chars:  See
+Read 4 chars:  the
+Read 4 chars:  Lic
+Read 4 chars: ense
+Read 4 chars:  for
+Read 4 chars:  the
+Read 4 chars:  spe
+Read 4 chars: cifi
+Read 4 chars: c la
+Read 4 chars: ngua
+Read 4 chars: ge g
+Read 4 chars: over
+Read 4 chars: ning
+Read 4 chars:  per
+Read 4 chars: miss
+Read 4 chars: ions
+Read 4 chars:  and
+Read 4 chars: 
+# l
+Read 4 chars: imit
+Read 4 chars: atio
+Read 4 chars: ns u
+Read 4 chars: nder
+Read 4 chars:  the
+Read 4 chars:  Lic
+Read 4 chars: ense
+Read 4 chars: .
+
+m
+Read 4 chars: odul
+Read 4 chars: e te
+Read 4 chars: st_a
+Read 4 chars: nnot
+Read 4 chars: _c_c
+Read 4 chars: ompi
+Read 4 chars: ler 
+Read 4 chars: is
+       
+Read 4 chars: cfla
+Read 4 chars: gs "
+Read 4 chars: -I /
+Read 4 chars: usr/
+Read 4 chars: incl
+Read 4 chars: ude"
+Read 4 chars: 
+       cf
+Read 4 chars: lags
+Read 4 chars:  exe
+Read 4 chars: c("p
+Read 4 chars: kg-c
+Read 4 chars: onfi
+Read 4 chars: g", 
+Read 4 chars: "--c
+Read 4 chars: flag
+Read 4 chars: s", 
+Read 4 chars: "sdl
+Read 4 chars: ")
+       
+Read 4 chars: ldfl
+Read 4 chars: ags 
+Read 4 chars: "-lm
+Read 4 chars: "
+       l
+Read 4 chars: dfla
+Read 4 chars: gs("
+Read 4 chars: -lm"
+Read 4 chars: , "-
+Read 4 chars: L /u
+Read 4 chars: sr/b
+Read 4 chars: in")
+Read 4 chars: 
+end
+Read 4 chars: 
+
+fu
+Read 4 chars: n du
+Read 4 chars: mmy 
+Read 4 chars: `{ p
+Read 4 chars: rint
+Read 4 chars: f("n
+Read 4 chars: othi
+Read 4 chars: ng..
+Read 4 chars: .\n"
+Read 4 chars: ); `
+Read 4 chars: }
+
+d
+Read 4 chars: ummy
+Read 1 chars: 
+
diff --git a/tests/test_bufferedfilereader.nit b/tests/test_bufferedfilereader.nit
new file mode 100644 (file)
index 0000000..f5ac80a
--- /dev/null
@@ -0,0 +1,22 @@
+# This file is part of NIT ( http://www.nitlanguage.org ).
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+var f = new FileReader.open("test_annot_c_compiler.nit")
+
+while not f.eof do
+       var s = f.read(4)
+       print "Read {s.length} chars: {s}"
+end
+
+f.close