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

Why all the read buffering? #13

Closed
xeoncross opened this issue Nov 3, 2016 · 2 comments
Closed

Why all the read buffering? #13

xeoncross opened this issue Nov 3, 2016 · 2 comments

Comments

@xeoncross
Copy link

xeoncross commented Nov 3, 2016

There seems to be a lot of buffering structs added. Why are they being used? Was the go defaults for reading from an io.Reader unsafe?

Are you trying to limit the amount read? You could use an io.LimitReader():

data, err := ioutil.ReadAll(io.LimitReader(client.conn, 1000000))
@flashmob
Copy link
Owner

flashmob commented Nov 3, 2016

There were two reasons;

  1. Wanted to 'extend' the buffio.Reader so that it has a limited reader underneath plus add a setLimit() method to adjust the limit. (Didn't want to allocate a new limitedreader each time)
  2. wanted to 'extend' the limited reader so that the limit could be adjusted. The other problem was that limit reader returned io.EOF when limit reached, but needed a more specific error

@flashmob
Copy link
Owner

flashmob commented Nov 3, 2016

Thanks for your review btw! After having a second look at it, it looks like the err handling should have been placed inside the limited reader, then there is no need for readLimitedString() - can just use good old bufio.readString()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants