JuliaIO / GZip.jl

A Julia interface for gzip functions in zlib
https://juliaio.github.io/GZip.jl/dev
MIT License
39 stars 30 forks source link

Set the EOF flag when opening empty files. #43

Closed gustafsson closed 8 years ago

gustafsson commented 9 years ago

This PR includes a test that fails prior to this PR. Below is another use case that fails before this PR:

;touch foo
open("foo") do f
    map(println,eachline(f))
end
# works fine

;gzip foo # creates foo.gz
GZip.open("foo.gz") do f
    map(println,eachline(f))
end
# exception
gustafsson commented 8 years ago

@yuyichao mind taking a look at this?

yuyichao commented 8 years ago

Errr, not sure why I'm pinged ;-p.....

LGTM, using peek to set EOF feels a little strange but seems to be how other places does it ....

gustafsson commented 8 years ago

I didn't know who was the maintainer of this repo but I saw your name in recent activity so I figured you'd be able to decide whether to do a merge or not.

The GZipStream interface assumes that you check eof() before calling read() rather than keep doing read() until you reach the special EOF character (which is the zlib interface). In order for eof() to work EOF must have first been reached, which is accomplished by a peek(). But this procedure missed the case where EOF is encountered on the very first read() before the EOF-bit is set in zlib.

So given the behaviour of the GZipStream interface, peek must be called prior to the first read.

yuyichao commented 8 years ago

Yeah I'm just staying using peak to set a flag feels a little counter-intuitive. It looks fine given this is how it is used elsewhere.

Anyway the change LGTM and the test looks good too. Given that no one have raised any comment in the past months I'll merge this now....