Which is the case that returning n > 0 and err != nil on n, err = r.Read(b) ?

[mattn]

Hi list.

See third block in http://golang.org/pkg/io/#Reader

> An instance of this general case is that a Reader returning a non-zero number of bytes at the end of the input stream may return either err == EOF or err == nil. The next Read should return 0, EOF regardless.

Which is the case that returning n > 0 and err != nil both on n, err = r.Read(b) ? Most of programmers expects err == nil when n > 0.

- Yasuhiro Matsumoto

[Jesse McNelis]

On Mon, Sep 1, 2014 at 3:07 PM, mattn <matt...@gmail.com> wrote:
> Which is the case that returning n > 0 and err != nil both on n, err =
> r.Read(b) ?
>Most of programmers expects err == nil when n > 0.

They shouldn't. n > 0 will often happen when err == io.EOF.
As recommended by the io.Reader documentation, you should always check
the value of 'n' and process those bytes before checking the error
value.

http://golang.org/pkg/io/#Reader

[mattn]

I want to know WHICH CASE make the error happen.

man read(2) says return code should be 0 if it at EOF. So as far as using os.File, it shouldn't be happen. I guess.

[Jan Mercl]

On Mon, Sep 1, 2014 at 8:44 AM, mattn <matt...@gmail.com> wrote:
> I want to know WHICH CASE make the error happen.

Any case the author of the code implementing the io.Reader finds
appropriate. The point is that the contract allows that, makes perfect
sense - and the client of an io.Reader must just cope with that
situation correctly.

A simple example is possibly the situation when the client asks for N
bytes and some non extensible buffer has exactly N unread bytes left
-> return N, io.EOF.

-j

[mattn]

I want to know which situation the OS API return such value and error both. I can say that It shelve an error code of ERROR_SUCCESS.

I want to say, all implementor mustn't return n > 0 and io.EOF both in same return. 

[mattn]

Go recommends the early return. So returning n > 0 and io.EOF at the same time is inconsistent, I think. To handle err easily, it hope to not return n > 0 and io.EOF at the same time.

n, err := r.Read(b)

if err != nil {

    return err

}

// do something, n should be greater than zero.

If io.Reader allow to given returning n > 0 and io.EOF both, we must write code like below.

n, err := r.Read(b)

if err != nil && err != io.EOF {

    return err

}

if n > 0 {

    // do something

}

return err

Thanks.

- Yasuhiro Matsumoto

[Jan Mercl]

On Mon, Sep 1, 2014 at 9:38 AM, mattn <matt...@gmail.com> wrote:
> Go recommends the early return. So returning n > 0 and io.EOF at the same
> time is inconsistent, I think. To handle err easily, it hope to not return n
>> 0 and io.EOF at the same time.

Go encourages to handle errors at the place where they're reported.
However, if the io.Reader.Read is defined to be free to return valid,
non zero length data _and_ a non nil error in the same time then the
correct client behavior is to consume the valid data first.

> n, err := r.Read(b)
> if err != nil {
> return err
> }
> // do something, n should be greater than zero.
>
> If io.Reader allow to given returning n > 0 and io.EOF both, we must write
> code like below.
>
> n, err := r.Read(b)
> if err != nil && err != io.EOF {
> return err
> }
> if n > 0 {
> // do something
> }
> return err

n, err := r.Read(b)

if n > 0 {
// consume b[:n]
}

if err != nil {
// handle err, probably treating io.EOF specially
}

Also, it's easy to wrap any io.Reader in a way where the "outer"
Reader will never return non zero n && non nil error while preserving
the correct semantics.

-j

[mattn]

On Monday, September 1, 2014 5:01:06 PM UTC+9, Jan Mercl wrote:

Go encourages to handle errors at the place where they're reported.
However, if the io.Reader.Read is defined to be free to return valid,
non zero length data _and_ a non nil error in the same time then the
correct client behavior is to consume the valid data first.

Yes, It's thing i wanted to say totally. And I want to know why go allow this. For example, below's code return n > 0 and io.EOF in same time.

http://play.golang.org/p/71RQP58qNo

 

n, err := r.Read(b)
if n > 0 {
        // consume b[:n]
}

if err != nil {
        // handle err, probably treating io.EOF specially
}

Also, it's easy to wrap any io.Reader in a way where the "outer"
Reader will never return non zero n && non nil error while preserving
the correct semantics.

I'm still having question why go had allow this. For example, in your code in above, when a lot of codes are need to consume b[:n], we will lost one if-nest of code. Or need to check n == 0 like below.

if n == 0 && err != nil {

    return err

}

// consume b[:n]

- Yasuhiro Matsumoto

[Jesse McNelis]

On Mon, Sep 1, 2014 at 5:38 PM, mattn <matt...@gmail.com> wrote:
> Go recommends the early return. So returning n > 0 and io.EOF at the same
> time is inconsistent, I think. To handle err easily, it hope to not return n
>> 0 and io.EOF at the same time.
>
> n, err := r.Read(b)
> if err != nil {
> return err
> }
> // do something, n should be greater than zero.

So any error wouldn't be returned by Read() when it happens, it would
instead be returned by the next call to Read? This requires whatever
you're reading from to remember the error for the caller, which means
you can't implement io.Reader with any type that doesn't have an error
field.

[Mateusz Czapliński]

On Monday, September 1, 2014 10:15:50 AM UTC+2, mattn wrote:

I'm still having question why go had allow this.

If I recall correctly, this seems to be unfortunate legacy behavior, which cannot be changed now because of Go 1 contract. But I may be wrong.

/M.

[Rui Ueyama]

A counterexample to considering it as an unfortunate legacy is recent Brad's commit to net/http: https://codereview.appspot.com/49570044. It changed the behavior of its Read to return data and EOF to reuse connections as soon as possible.

We have to handle such cases correctly because it's a part of the io.Reader's contract. I'd personally want more Readers to return data and EOF so that I can find buggy code (which assumes data will never be returned with an error) earlier.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[robfig]

Does the tar archive reader behave similarly? I think it implies that it will not return N>0, io.EOF, but it is not specific about it.

http://godoc.org/archive/tar#Reader.Next
"Read reads from the current entry in the tar archive. It returns 0, io.EOF when it reaches the end of that entry, until Next is called to advance to the next entry."

I wouldn't be surprised if a large percent of non-stdlib Reader consumers get the pattern wrong. As mattn says, this case is different from every other Go API, so the muscle memory may overpower the details in the spec, and depending on answer to above question, it seems like other stdlib reader-ish things do not behave similarly.

Would checking for this mistaken usage be reasonable to add to "go vet" ? Perhaps vet could alert on the following case, which seems likely to be the most common incorrect usage pattern:

_, err := r.Read(b)
if err == io.EOF {
break
}
...

Not sure if that's within the scope of "go vet", or if it has the requisite type information.

[Konstantin Khomoutov]

On Mon, 1 Sep 2014 10:00:10 +0200
Jan Mercl <0xj...@gmail.com> wrote:

> On Mon, Sep 1, 2014 at 9:38 AM, mattn <matt...@gmail.com> wrote:
> > Go recommends the early return. So returning n > 0 and io.EOF at
> > the same time is inconsistent, I think. To handle err easily, it
> > hope to not return n
> >> 0 and io.EOF at the same time.
>
> Go encourages to handle errors at the place where they're reported.
> However, if the io.Reader.Read is defined to be free to return valid,
> non zero length data _and_ a non nil error in the same time then the
> correct client behavior is to consume the valid data first.

I would add that io.Reader.Read() appears to be actually modelled after
the read(2) Unix syscall, and people who find this behaviour
counter-intuitive should possibly just use io.ReadFull() instead and
check for the returned error value being io.UnexpectedEOF.

Or, to put it differently, there are in fact just two ways of
interpreting the data being read: either you know how much bytes you
will read or you don't. io.ReadFull() is for the former case, and
io.Reader.Read() is for the latter, plus "advanced" cases like, say,
implementing a HTTP downloader with resume support which should save
all the data received even if the stream has been terminated earlier
than the expected EOF due to an error. And for that latter case
employing io.Copy() appears to be an even better idea as it deals with
that io.Reader.Read()'s imaginary inconsistency by itself just okay.

[Rui Ueyama]

I think writing code to find this pattern and running it against the public code base would be interesting. We'd see how much code is wrong.

[Raffaele Sena]

I actually had this problem recently and suspect that the archive packages get this wrong.

I have implemented an HttpFile reader (http://godoc.org/github.com/gobs/httpclient#HttpFile), that allows you to "seek" and read from a remote file (it does Range requests according to current file position) and then implemented a "streaming unzip" service (a thing that allows you to ask for a file archived in a remote zip file) that simply passes the HttpFile reader to the zip reader.

Well, it failed right away trying to read the directory (the zip reader does a bunch of seeks to figure out the file length, read the directory offset at EOF and then read the directory). And the problem was the http request at "filesize-4" used to read the directory offset returning the requested 4 bytes AND io.EOF.

I solved by making my read always return (n, nil) or (0, io.EOF) and meant to report the error (the zip reader deals incorrectly with n, io.EOF) but didn't get to it.

-- Raffaele

[Rui Ueyama]

I had a quick look at archive/zip. There's only one Read call in the package, and the code seemed good to me. I didn't run it though.

However there are many ReadAt calls that look suspicious there. Are you talking about them?