Skip to content

Commit

Permalink
fix #9687, fix #8496
Browse files Browse the repository at this point in the history
- in eof(::IOStream) always check whether we are really still at EOF
- add `all` option to readbytes[!]. `all=false` exposes short reads to
  the caller.
- use `filesize` in readbytes(::IOStream) to avoid resizing/moving
  during reading. much faster for big files.
  • Loading branch information
JeffBezanson committed Jul 12, 2015
1 parent a667d2b commit dcb65f7
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 15 deletions.
44 changes: 39 additions & 5 deletions base/iostream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -214,24 +214,58 @@ function readuntil(s::IOStream, delim::UInt8)
ccall(:jl_readuntil, Array{UInt8,1}, (Ptr{Void}, UInt8), s.ios, delim)
end

function readbytes!(s::IOStream, b::Array{UInt8}, nb=length(b))
function readbytes_all!(s::IOStream, b::Array{UInt8}, nb)
olb = lb = length(b)
nr = 0
while !eof(s) && nr < nb
while nr < nb
if lb < nr+1
lb = max(65536, (nr+1) * 2)
resize!(b, lb)
end
nr += Int(ccall(:ios_readall, UInt,
(Ptr{Void}, Ptr{Void}, UInt),
nr += Int(ccall(:ios_readall, UInt, (Ptr{Void}, Ptr{Void}, UInt),
s.ios, pointer(b, nr+1), min(lb-nr, nb-nr)))
eof(s) && break
end
if lb > olb
if lb > olb && lb > nr
resize!(b, nr) # shrink to just contain input data if was resized
end
return nr
end

function readbytes_some!(s::IOStream, b::Array{UInt8}, nb)
olb = lb = length(b)
if nb > lb
resize!(b, nb)
end
nr = Int(ccall(:ios_read, UInt, (Ptr{Void}, Ptr{Void}, UInt),
s.ios, pointer(b), nb))
if lb > olb && lb > nr
resize!(b, nr)
end
return nr
end

function readbytes!(s::IOStream, b::Array{UInt8}, nb=length(b); all::Bool=true)
return all ? readbytes_all!(s, b, nb) : readbytes_some!(s, b, nb)
end

function readbytes(s::IOStream)
sz = filesize(s)
pos = ccall(:ios_pos, FileOffset, (Ptr{Void},), s.ios)
if pos > 0
sz -= pos
end
b = Array(UInt8, sz<=0 ? 1024 : sz)
nr = readbytes_all!(s, b, typemax(Int))
resize!(b, nr)
end

function readbytes(s::IOStream, nb::Integer; all::Bool=true)
b = Array(UInt8, nb)
nr = readbytes!(s, b, nb, all=all)
resize!(b, nr)
end

## Character streams ##
const _chtmp = Array(Char, 1)
function peekchar(s::IOStream)
Expand Down
16 changes: 12 additions & 4 deletions doc/stdlib/io-network.rst
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,23 @@ General I/O

Read binary data from a stream, filling in the argument ``array``.

.. function:: readbytes!(stream, b::Vector{UInt8}, nb=length(b))
.. function:: readbytes!(stream, b::Vector{UInt8}, nb=length(b); all=true)

Read at most ``nb`` bytes from the stream into ``b``, returning the
number of bytes read (increasing the size of ``b`` as needed).

.. function:: readbytes(stream, nb=typemax(Int))
See ``readbytes`` for a description of the ``all`` option.

Read at most ``nb`` bytes from the stream, returning a
``Vector{UInt8}`` of the bytes read.
.. function:: readbytes(stream, nb=typemax(Int); all=true)

Read at most ``nb`` bytes from the stream, returning a ``Vector{UInt8}`` of the bytes read.

If ``all`` is true (the default), this function will block repeatedly
trying to read all requested bytes, until an error or end-of-file
occurs.
If ``all`` is false, at most one ``read`` call is performed, and the
amount of data returned is device-dependent.
Note that not all stream types support the ``all`` option.

.. function:: position(s)

Expand Down
11 changes: 5 additions & 6 deletions src/support/ios.c
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ static size_t _ios_read(ios_t *s, char *dest, size_t n, int all)
{
size_t tot = 0;
size_t got, avail;
//int result;
int didread = 0;

if (s->state == bst_wr) {
ios_seek(s, ios_pos(s));
Expand All @@ -256,9 +256,8 @@ static size_t _ios_read(ios_t *s, char *dest, size_t n, int all)
size_t ncopy = (avail >= n) ? n : avail;
memcpy(dest, s->buf + s->bpos, ncopy);
s->bpos += ncopy;
if (ncopy >= n) {
if (ncopy >= n)
return tot+ncopy;
}
}
if (s->bm == bm_mem || s->fd == -1) {
// can't get any more data
Expand All @@ -271,9 +270,10 @@ static size_t _ios_read(ios_t *s, char *dest, size_t n, int all)
n -= avail;
tot += avail;

if (didread && !all) return tot;

ios_flush(s);
s->bpos = s->size = 0;

s->fpos = -1;
if (n > MOST_OF(s->maxsize)) {
// doesn't fit comfortably in buffer; go direct
Expand Down Expand Up @@ -302,6 +302,7 @@ static size_t _ios_read(ios_t *s, char *dest, size_t n, int all)
}
s->size = got;
}
didread = 1;
}

return tot;
Expand Down Expand Up @@ -580,8 +581,6 @@ int ios_eof_blocking(ios_t *s)
return (s->_eof ? 1 : 0);
if (s->fd == -1)
return 1;
if (s->_eof)
return 1;

if (ios_readprep(s, 1) < 1)
return 1;
Expand Down
13 changes: 13 additions & 0 deletions test/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -889,3 +889,16 @@ rm(dir)
# The following fail on Windows with "stat: operation not permitted (EPERM)"
@unix_only @test !ispath(file)
@unix_only @test !ispath(dir)

# issue #9687
let n = tempname()
w = open(n, "a")
io = open(n)
write(w, "A"); flush(w)
@test readbytes(io) == UInt8[0x41]
@test readbytes(io) == UInt8[]
write(w, "A"); flush(w)
@test readbytes(io) == UInt8[0x41]
close(io); close(w)
rm(n)
end

1 comment on commit dcb65f7

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.