Skip to content

Commit

Permalink
Multiple fixes...
Browse files Browse the repository at this point in the history
1. Fixed the #seek inheritance hierarchy mostly by no longer
conflating #seek and #sysseek as the same thing.
2. Rename PipeFileDescriptor to FIFOFileDescriptor since it
can also be used for special character files.
3. Fix #gets so the peek ahead for multi-byte characters is
properly respected; refill the buffer if we don't have enough
bytes for peek ahead.
4. Fix important error in #new_open_fd which was using the wrong
flag on an FD. When using fcntl on an FD, it is important to use
the FD_* flags. Was using O_CLOEXEC instead of FD_CLOEXEC and as
a result all child processes were hanging when their read pipe
was closed.
5. Handle EPIPE in #write correctly. Also raise an error if we
get an error instead of ignoring it.
chuckremes committed Feb 6, 2015
1 parent c259427 commit 8af5016
Showing 1 changed file with 59 additions and 39 deletions.
98 changes: 59 additions & 39 deletions kernel/common/io.rb
Original file line number Diff line number Diff line change
@@ -48,9 +48,16 @@ def self.choose_type(fd)
case stat.ftype
when "file"
BufferedFileDescriptor.new(fd, stat)
when "pipe"
PipeFileDescriptor.new(fd)
when "fifo", "characterSpecial"
FIFOFileDescriptor.new(fd, stat)
when "socket"
raise "cannot make socket yet"
when "directory"
raise "cannot make a directory"
when "blockSpecial"
raise "cannot make block special"
when "link"
raise "cannot make link"
else
new(fd, stat)
end
@@ -82,7 +89,7 @@ def self.new_open_fd(new_fd)
if new_fd > 2
flags = FFI::Platform::POSIX.fcntl(new_fd, F_GETFD, 0)
Errno.handle("fcntl(2) failed") if FFI.call_failed?(flags)
flags = FFI::Platform::POSIX.fcntl(new_fd, F_SETFD, FFI::Platform::POSIX.fcntl(new_fd, F_GETFL, 0) | O_CLOEXEC)
flags = FFI::Platform::POSIX.fcntl(new_fd, F_SETFD, FFI::Platform::POSIX.fcntl(new_fd, F_GETFL, 0) | FD_CLOEXEC)
Errno.handle("fcntl(2) failed") if FFI.call_failed?(flags)
end

@@ -114,7 +121,6 @@ def initialize(fd, stat)
end

@sync = true

reset_positioning(@stat)

# Don't bother to add finalization for stdio
@@ -149,7 +155,7 @@ def sync=(value)

CLONG_OVERFLOW = 1 << 64

def sysseek(offset, whence=SEEK_SET)
def lseek(offset, whence=SEEK_SET)
ensure_open

# FIXME: check +amount+ to make sure it isn't too large
@@ -243,12 +249,12 @@ def write(str)
continue
elsif errno == Errno::EPIPE::Errno
if @descriptor == 1 || @descriptor == 2
return buf_size
return(buf_size)
end
else
error = true
break
end

error = true
break
end

break if error
@@ -257,6 +263,8 @@ def write(str)
buffer += bytes_written
@offset += bytes_written
end

Errno.handle("write failed") if error

return(buf_size - left)
end
@@ -409,7 +417,7 @@ def reset_positioning(stat=nil)
end

def seek_positioning
@offset = sysseek(0, SEEK_CUR) # find current position if we are reopening!
@offset = lseek(0, SEEK_CUR) # find current position if we are reopening!
end
private :seek_positioning

@@ -454,6 +462,11 @@ def tty?
end # class FileDescriptor

class BufferedFileDescriptor < FileDescriptor

def buffer_reset
@unget_buffer.clear
end
private :buffer_reset

def read(length, output_string=nil)
length ||= FileDescriptor.pagesize
@@ -474,13 +487,13 @@ def read(length, output_string=nil)
elsif str2
str += str2
end
@unget_buffer.clear
buffer_reset
elsif length == @unget_buffer.size
@offset += length
length -= @unget_buffer.size

str = @unget_buffer.inject("".force_encoding(Encoding::ASCII_8BIT)) { |sum, val| val.chr + sum }
@unget_buffer.clear
buffer_reset
else
@offset += @unget_buffer.size
str = "".force_encoding(Encoding::ASCII_8BIT)
@@ -500,27 +513,42 @@ def read(length, output_string=nil)
return output_string
end

def seek(bytes, whence)
# @offset may not match actual file pointer if there were calls to #unget.
if whence == SEEK_CUR
# adjust the number of bytes to seek based on how far ahead we are with the buffer
bytes -= @unget_buffer.size
end

buffer_reset
lseek(bytes, whence)
end

def sysread(byte_count)
STDERR.puts "sysread [#{byte_count}], @unget_buffer #{@unget_buffer.inspect}"
raise_if_buffering
read(byte_count)
end

def sysseek(bytes, whence)
raise_if_buffering
lseek(bytes, whence)
end

def eof?
super && @unget_buffer.empty?
end

def flush
@unget_buffer.clear
#@unget_buffer.clear
end

def raise_if_buffering
raise IOError unless @unget_buffer.empty?
end

def reset_positioning(*args)
super
@unget_buffer = []
super
end

def unget(byte)
@@ -529,7 +557,7 @@ def unget(byte)
end
end # class BufferedFileDescriptor

class PipeFileDescriptor < BufferedFileDescriptor
class FIFOFileDescriptor < BufferedFileDescriptor

def self.connect_pipe_fds
fds = FFI::MemoryPointer.new(:int, 2)
@@ -543,11 +571,9 @@ def self.connect_pipe_fds
return [fd0, fd1]
end

def initialize(fd, mode)
@descriptor = fd
@mode = mode
@sync = true
reset_positioning
def initialize(fd, stat, mode=nil)
super(fd, stat)
@mode = mode if mode
@eof = false # force to false
end

@@ -575,24 +601,20 @@ def seek_positioning
end

def sysseek(offset, whence=SEEK_SET)
ensure_open

# lseek does not work with pipes.
raise Errno::ESPIPE
end
end # class PipeFileDescriptor
end # class FIFOFileDescriptor

def new_pipe(fd, external, internal, options, mode, do_encoding=false)
@fd = PipeFileDescriptor.new(fd, mode)
@fd = FIFOFileDescriptor.new(fd, nil, mode)
@lineno = 0
@pipe = true

# Why do we only set encoding for the "left hand side" pipe? Why not both?
if do_encoding
set_encoding((external || Encoding.default_external), (internal || Encoding.default_internal), options)
end

# setup finalization for pipes, FIXME
end
private :new_pipe

@@ -1035,7 +1057,7 @@ def self.pipe(external=nil, internal=nil, options=nil)
# The use of #allocate is to make sure we create an IO obj. Would be so much
# cleaner to just do a PipeIO class as a subclass, but that would not be
# backward compatible. <sigh>
fd0, fd1 = PipeFileDescriptor.connect_pipe_fds
fd0, fd1 = FIFOFileDescriptor.connect_pipe_fds
lhs = allocate
lhs.send(:new_pipe, fd0, external, internal, options, FileDescriptor::O_RDONLY, true)
rhs = allocate
@@ -1143,7 +1165,7 @@ def self.popen(*args)
else
return nil
end
rescue
rescue => e
exit! 0
end
end
@@ -1556,6 +1578,8 @@ def dup
#

class EachReader
READ_SIZE = 512 # bytes

def initialize(io, separator, limit)
@io = io
@separator = separator ? separator.force_encoding("ASCII-8BIT") : separator
@@ -1605,7 +1629,7 @@ def read_to_separator

until buffer.size == 0 && @io.eof?
if buffer.size == 0
@io.read(PEEK_AHEAD_LIMIT + 2, buffer)
buffer = @io.read(READ_SIZE)
end

break unless buffer.size > 0
@@ -1691,7 +1715,7 @@ def read_to_separator_with_limit

until buffer.size == 0 && @io.eof?
if buffer.size == 0
buffer = @io.read(PEEK_AHEAD_LIMIT + 2)
buffer = @io.read(READ_SIZE)
end

break unless buffer && buffer.size > 0
@@ -1719,6 +1743,8 @@ def read_to_separator_with_limit
if wanted < buffer.size
str << buffer.slice!(0, wanted)

# replenish the buffer if we don't have enough bytes to satisfy the peek ahead
buffer += @io.read(PEEK_AHEAD_LIMIT) if buffer.size < PEEK_AHEAD_LIMIT
str, bytes_read = read_to_char_boundary(@io, str, buffer)
str.taint

@@ -2012,7 +2038,7 @@ def fileno
# no newline
def flush
ensure_open
@fd.reset_positioning
#@fd.reset_positioning
return self
end

@@ -2564,7 +2590,7 @@ def seek(amount, whence=SEEK_SET)

@eof = false

@fd.sysseek Integer(amount), whence
@fd.seek Integer(amount), whence

return 0
end
@@ -2763,12 +2789,6 @@ def sysread(number_of_bytes, buffer=undefined)
# f.sysread(10) #=> "And so on."
def sysseek(amount, whence=SEEK_SET)
ensure_open
# if @ibuffer.write_synced?
# raise IOError unless @ibuffer.empty?
# else
# warn 'sysseek for buffered IO'
# end

amount = Integer(amount)

@fd.sysseek amount, whence

0 comments on commit 8af5016

Please sign in to comment.