Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix eof() and read*() behaviour for ::File and ::LibuvStream #14699

Merged
merged 7 commits into from
Jan 24, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions base/filesystem.jl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export File,
S_IROTH, S_IWOTH, S_IXOTH, S_IRWXO

import Base: uvtype, uvhandle, eventloop, fd, position, stat, close,
write, read, read!, isopen, show,
write, read, readavailable, read!, isopen, show,
seek, seekend, skip, eof,
check_open, _sizeof_uv_fs, uv_error, UVError

include("path.jl")
Expand Down Expand Up @@ -158,27 +159,53 @@ function read!(f::File, a::Vector{UInt8}, nel=length(a))
end
ret = ccall(:jl_fs_read, Int32, (Int32, Ptr{Void}, Csize_t),
f.handle, a, nel)
if ret < nel
throw(EOFError())
end
uv_error("read",ret)
return a
end

nb_available(f::File) = filesize(f) - position(f)

eof(f::File) = nb_available(f) == 0

function readbytes!(f::File, b::Array{UInt8}, nb=length(b))
nr = min(nb, nb_available(f))
if length(b) < nr
resize!(b, nr)
end
read!(f, b, nr)
return nr
ret = ccall(:jl_fs_read, Int32, (Int32, Ptr{Void}, Csize_t),
f.handle, b, nr)
uv_error("read",ret)
return ret
end
read(io::File) = read!(io, Array(UInt8, nb_available(io)))
readavailable(io::File) = read(io)
read(io::File, nb::Integer) = read!(io, Array(UInt8, min(nb, nb_available(io))))

const SEEK_SET = Int32(0)
const SEEK_CUR = Int32(1)
const SEEK_END = Int32(2)

function seek(f::File, n::Integer)
ret = ccall(:jl_lseek, Int64, (Int32, Int64, Int32), f.handle, n, SEEK_SET)
systemerror("seek", ret == -1)
return f
end

function seekend(f::File)
ret = ccall(:jl_lseek, Int64, (Int32, Int64, Int32), f.handle, 0, SEEK_END)
systemerror("seekend", ret == -1)
return f
end

function skip(f::File, n::Integer)
ret = ccall(:jl_lseek, Int64, (Int32, Int64, Int32), f.handle, n, SEEK_CUR)
systemerror("skip", ret == -1)
return f
end

function position(f::File)
check_open(f)
ret = ccall(:jl_lseek, Int64, (Int32, Int64, Int32), f.handle, 0, SEEK_CUR)
Expand Down
5 changes: 4 additions & 1 deletion base/io.jl
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,10 @@ type EachLine
EachLine(stream, ondone) = new(stream, ondone)
end
eachline(stream::IO) = EachLine(stream)
eachline(filename::AbstractString) = EachLine(open(filename), close)
function eachline(filename::AbstractString)
s = open(filename)
EachLine(s, ()->close(s))
end

start(itr::EachLine) = nothing
function done(itr::EachLine, nada)
Expand Down
1 change: 1 addition & 0 deletions base/iobuffer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ function readbytes!(io::AbstractIOBuffer, b::Array{UInt8}, nb=length(b))
return nr
end
read(io::AbstractIOBuffer) = read!(io, Array(UInt8, nb_available(io)))
readavailable(io::AbstractIOBuffer) = read(io)
read(io::AbstractIOBuffer, nb::Integer) = read!(io, Array(UInt8, min(nb, nb_available(io))))

function search(buf::IOBuffer, delim::UInt8)
Expand Down
2 changes: 2 additions & 0 deletions base/iostream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ end
# num bytes available without blocking
nb_available(s::IOStream) = ccall(:jl_nb_available, Int32, (Ptr{Void},), s.ios)

readavailable(s::IOStream) = read!(s, Vector{UInt8}(nb_available(s)))

function read(s::IOStream, ::Type{UInt8})
b = ccall(:ios_getc, Cint, (Ptr{Void},), s.ios)
if b == -1
Expand Down
23 changes: 12 additions & 11 deletions base/stream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -894,32 +894,32 @@ function stop_reading(stream::LibuvStream)
end
end

function readbytes!(s::LibuvStream, b::AbstractArray{UInt8}, nb=length(b))
wait_readnb(s, nb)
nr = nb_available(s)
resize!(b, nr) # shrink to just contain input data if was resized
read!(s.buffer, b)
return nr
function read!(s::LibuvStream, b::Vector{UInt8})
nb = length(b)
r = readbytes!(s, b, nb)
if r < nb
throw(EOFError())
end
return b
end

function read(stream::LibuvStream)
wait_readnb(stream, typemax(Int))
return takebuf_array(stream.buffer)
end

function read!(s::LibuvStream, a::Array{UInt8, 1})
nb = length(a)
function readbytes!(s::LibuvStream, a::Vector{UInt8}, nb = length(a))
sbuf = s.buffer
@assert sbuf.seekable == false
@assert sbuf.maxsize >= nb

if nb_available(sbuf) >= nb
return read!(sbuf, a)
return readbytes!(sbuf, a, nb)
end

if nb <= SZ_UNBUFFERED_IO # Under this limit we are OK with copying the array from the stream's buffer
wait_readnb(s, nb)
read!(sbuf, a)
r = readbytes!(sbuf, a, nb)
else
try
stop_reading(s) # Just playing it safe, since we are going to switch buffers.
Expand All @@ -928,14 +928,15 @@ function read!(s::LibuvStream, a::Array{UInt8, 1})
s.buffer = newbuf
write(newbuf, sbuf)
wait_readnb(s, nb)
r = nb_available(newbuf)
finally
s.buffer = sbuf
if !isempty(s.readnotify.waitq)
start_reading(s) # resume reading iff there are currently other read clients of the stream
end
end
end
return a
return r
end

function read(this::LibuvStream, ::Type{UInt8})
Expand Down
8 changes: 8 additions & 0 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,14 @@ JL_DLLEXPORT void JL_NORETURN jl_bounds_error_ints(jl_value_t *v, size_t *idxs,
jl_throw(jl_new_struct((jl_datatype_t*)jl_boundserror_type, v, t));
}

JL_DLLEXPORT void JL_NORETURN jl_eof_error(void)
{
jl_datatype_t *eof_error =
(jl_datatype_t*)jl_get_global(jl_base_module, jl_symbol("EOFError"));
assert(eof_error != NULL);
jl_exceptionf(eof_error, "");
}

JL_CALLABLE(jl_f_throw)
{
JL_NARGS(throw, 1, 1);
Expand Down
9 changes: 6 additions & 3 deletions src/jl_uv.c
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,12 @@ JL_DLLEXPORT int jl_fs_read_byte(int handle)
buf[0].len = 1;
int ret = uv_fs_read(jl_io_loop, &req, handle, buf, 1, -1, NULL);
uv_fs_req_cleanup(&req);
if (ret == -1)
return ret;
return (int)c;
switch (ret) {
case -1: return ret;
case 0: jl_eof_error();
case 1: return (int)c;
default: assert(0);
}
}

JL_DLLEXPORT int jl_fs_close(int handle)
Expand Down
1 change: 1 addition & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,7 @@ JL_DLLEXPORT void JL_NORETURN jl_bounds_error_tuple_int(jl_value_t **v,
size_t nv, size_t i);
JL_DLLEXPORT void JL_NORETURN jl_bounds_error_unboxed_int(void *v, jl_value_t *vt, size_t i);
JL_DLLEXPORT void JL_NORETURN jl_bounds_error_ints(jl_value_t *v, size_t *idxs, size_t nidxs);
JL_DLLEXPORT void JL_NORETURN jl_eof_error(void);
JL_DLLEXPORT jl_value_t *jl_exception_occurred(void);
JL_DLLEXPORT void jl_exception_clear(void);

Expand Down
9 changes: 1 addition & 8 deletions src/sys.c
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,6 @@ JL_DLLEXPORT jl_value_t *jl_readuntil(ios_t *s, uint8_t delim)
return (jl_value_t*)a;
}

static void JL_NORETURN throw_eof_error(void)
{
jl_datatype_t *eof_error = (jl_datatype_t*)jl_get_global(jl_base_module, jl_symbol("EOFError"));
assert(eof_error != NULL);
jl_exceptionf(eof_error, "");
}

JL_DLLEXPORT uint64_t jl_ios_get_nbyte_int(ios_t *s, const size_t n)
{
assert(n <= 8);
Expand All @@ -323,7 +316,7 @@ JL_DLLEXPORT uint64_t jl_ios_get_nbyte_int(ios_t *s, const size_t n)
space = (size_t)(s->size - s->bpos);
ret = ios_readprep(s, n);
if (space == ret && ret < n)
throw_eof_error();
jl_eof_error();
} while(ret < n);
uint64_t x = 0;
uint8_t *buf = (uint8_t*)&s->buf[s->bpos];
Expand Down
2 changes: 1 addition & 1 deletion test/choosetests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function choosetests(choices = [])
"bitarray", "copy", "math", "fastmath", "functional",
"operators", "path", "ccall", "parse", "loading",
"bigint", "sorting", "statistics", "spawn", "backtrace",
"priorityqueue", "file", "mmap", "version", "resolve",
"priorityqueue", "file", "read", "mmap", "version", "resolve",
"pollfd", "mpfr", "broadcast", "complex", "socket",
"floatapprox", "datafmt", "reflection", "regex", "float16",
"combinatorics", "sysinfo", "rounding", "ranges", "mod2pi",
Expand Down
33 changes: 33 additions & 0 deletions test/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1115,3 +1115,36 @@ end
@test copy(DevNull) === DevNull
@test eof(DevNull)
@test print(DevNull, "go to /dev/null") === nothing

# Filesystem.File
tmpdir = mktempdir() do d
f = joinpath(d, "test.txt")
open(io->write(io, "123"), f, "w")
f1 = open(f)
f2 = Base.Filesystem.open(f, Base.Filesystem.JL_O_RDONLY)
@test read(f1, UInt8) == read(f2, UInt8)
@test read(f1, UInt8) == read(f2, UInt8)
@test read(f1, UInt8) == read(f2, UInt8)
@test_throws EOFError read(f1, UInt8)
@test_throws EOFError read(f2, UInt8)
close(f1)
close(f2)

a = UInt8[0,0,0]
f1 = open(f)
f2 = Base.Filesystem.open(f, Base.Filesystem.JL_O_RDONLY)
@test read!(f1, a) == read!(f2, a)
@test_throws EOFError read!(f1, a)
@test_throws EOFError read!(f2, a)
close(f1)
close(f2)

a = UInt8[0,0,0,0]
f1 = open(f)
f2 = Base.Filesystem.open(f, Base.Filesystem.JL_O_RDONLY)
@test_throws EOFError read!(f1, a)
@test_throws EOFError read!(f2, a)
close(f1)
close(f2)
end

Loading