reader: rewrite nextFrame to fix various bugs.

mreiferson / go-snappystream

a Go package for framed snappy streams

MIT License

45 stars 13 forks source link

reader: rewrite nextFrame to fix various bugs. #13

Closed bmatsuo closed 10 years ago

bmatsuo commented 10 years ago

Fixes #5 along with other bugs related to chunk size and handling of EOF.

compressed blocks may have length up to snappy.MaxEncodedLen(MaxBlockSize), the longest block which could successfully decode to MaxBlockSize or fewer bytes. blocks are checked to have decoded length at most MaxBlockSize before being decoded.
successfully decodes padding and skippable chunks with size up to 2^24-1, the maximum representable chunk size.
fixes error reporting for reserved unskippable blocks with length greater than MaxBlockSize up to 2^24 -1. instead of being reported as too long they are reported as unskippable blocks.
~~simplify reader tests.~~ (edit: I ended up leaving the existing tests mostly the same)
fix occurrences of the reader returning EOF after partial decoding.

bmatsuo commented 10 years ago

Performance appears to be approximately the same as master. The choice of ~~io.Copy~~ (edit: io.CopyN) is probably not the most performant, it will create its own buffers, but its use is not on hot code paths so I didn't optimize it.

bmatsuo commented 10 years ago

Sorry. The description in the pull requests was slightly incorrect. I made no substantial changes to the existing tests. So you can feel somewhat safer knowing that what was being tested before is still being tested in the same way.

mreiferson commented 10 years ago

nice work @bmatsuo, thanks.

Will look it over more closely...

mreiferson commented 10 years ago

last thing I'm thinking through is this EOF business...

bmatsuo commented 10 years ago

@mreiferson what is giving you pause about the EOF changes? The reasoning or the implementation (which basically amounts to the "noeof" and "noeof64" functions I added)?

mreiferson commented 10 years ago

OK, I'm good.... nice work :+1:

Want to squash?

bmatsuo commented 10 years ago

Squashed.

Thanks for putting up with me @mreiferson. I'm just super excited about snappy right now. I want this package to be rock solid and rip through streams lightspeed. You know? :smile:

mreiferson commented 10 years ago

hah, np, thanks for fixing up my shitty code :100: