Read never returns io.EOF / io.UnexpectedEOF

[simonferquel]

Expected behavior

client.Read should have the same semantic as an io.Reader when reaching the end of a file. RRead does not contain an error code, but if its count is 0, we should assume we reached the end of file and return either (0, io.EOF) or (0, io.UnexpectedEOF)

Current behavior

Reading from the EOF, returns (0, nil)

[stevvooe]

30 now has Read return io.EOF. when len(Rread.Data) == 0, but doesn't handle a short read correctly, which is a requirement of io.ReaderAt.

[simonferquel]

From what I read about io.Reader, it is a correct behavior to defer io.EOF until there is 0 bytes to read. So I think it is good enough.

[stevvooe]

@simonferquel We actually need to maintain io.ReaderAt semantics, since Session.Read takes an offset. I am not sure the effect of returning short reads from io.ReaderAt without an error.

[simonferquel]

I don't think we can handle the readat contract without handling the chunking inside client.Read (and if we do that, I would expect client.Write to do the chunking as well)

On Thu, Nov 17, 2016 at 12:30 AM +0100, "Stephen Day" notifications@github.com wrote:

@simonferquel We actually need to maintain io.ReaderAt semantics, since Session.Read takes an offset. I am not sure the effect of returning short reads from io.ReaderAt without an error.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[stevvooe]

We can return an error when the buffer is partially read. I omitted it in #30 because it would require a little bit of gymnastics to make the truncated request visible.

I also think there is an avenue to address this by creating a better negotiation process. Right now, it is a little monolithic and hides the msize result. Basically, before session instantiation, the codec and channel need to coordinate to provide the caller with a buffer size. There is a missing object on client that should negotiate and provide context for further operations.

[simonferquel]

That is the approach I took with my previous PR : make the client expose a MaxReadBufferSize and MaxWriteBufferSize.

[stevvooe]

@simonferquel Not really: the difference is that there is a separate object managing the negotiation. The channel itself is actually a product of the negotiation.