Permalink
Browse files

Fix buffering bug in the 'read' builtin.

The last test case in builtin-io.test.sh now passes.
  • Loading branch information...
Andy Chu
Andy Chu committed Jan 12, 2018
1 parent ad5fd9d commit 6456db09f82cbd622eefea45787b866d98fad32c
Showing with 47 additions and 2 deletions.
  1. +18 −1 core/builtin.py
  2. +28 −0 core/process_test.py
  3. +1 −1 test/spec.sh
View
@@ -485,6 +485,23 @@ def _AppendParts(s, spans, max_results, join_next, parts):
READ_SPEC.ShortFlag('-n', args.Int)
# sys.stdin.readline() in Python has buffering! TODO: Rewrite this tight loop
# in C? Less garbage probably.
# NOTE that dash, mksh, and zsh all read a single byte at a time. It appears
# to be required by POSIX? Could try libc getline and make this an option.
def ReadLineFromStdin():
chars = []
while True:
c = os.read(0, 1)
if not c:
break
chars.append(c)
if c == '\n':
break
return ''.join(chars)
def Read(argv, splitter, mem):
arg, i = READ_SPEC.Parse(argv)
@@ -513,7 +530,7 @@ def Read(argv, splitter, mem):
parts = []
join_next = False
while True:
line = sys.stdin.readline()
line = ReadLineFromStdin()
#log('LINE %r', line)
if not line: # EOF
status = 1
View
@@ -8,9 +8,11 @@
import unittest
from core.id_kind import Id
from core import builtin
from osh import ast_ as ast
from core import process # module under test
from core import runtime
from core import util
from core.cmd_exec_test import InitExecutor # helper
@@ -34,6 +36,32 @@ def _ExtProc(argv):
class ProcessTest(unittest.TestCase):
def testStdinRedirect(self):
waiter = process.Waiter()
fd_state = process.FdState()
PATH = '_tmp/one-two.txt'
# Write two lines
with open(PATH, 'w') as f:
f.write('one\ntwo\n')
# Should get the first line twice, because Pop() closes it!
r = runtime.PathRedirect(Id.Redir_Less, 0, PATH)
fd_state.Push([r], waiter)
#line1 = sys.stdin.readline()
line1 = builtin.ReadLineFromStdin()
fd_state.Pop()
fd_state.Push([r], waiter)
#line2 = sys.stdin.readline()
line2 = builtin.ReadLineFromStdin()
fd_state.Pop()
# sys.stdin.readline() would erroneously return 'two' because of buffering.
self.assertEqual('one\n', line1)
self.assertEqual('one\n', line2)
def testProcess(self):
# 3 fds. Does Python open it? Shell seems to have it too. Maybe it
View
@@ -261,7 +261,7 @@ builtins() {
}
builtin-io() {
sh-spec spec/builtin-io.test.sh --osh-failures-allowed 1 \
sh-spec spec/builtin-io.test.sh \
${REF_SHELLS[@]} $ZSH $BUSYBOX_ASH $OSH "$@"
}

0 comments on commit 6456db0

Please sign in to comment.