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

Decode() may return nil when output length is 0 (patch attached) #6

Open
GoogleCodeExporter opened this issue Mar 20, 2015 · 4 comments

Comments

@GoogleCodeExporter
Copy link

If Decode() is called with a nil destination buffer and the uncompressed length 
is zero, then a nil slice will be returned. This contradicts the docs, which 
say that "a newly allocated slice will be returned" in this case.

Repro: http://play.golang.org/p/NiNH49J4M2 . This doesn't actually run because 
the playground can't import snappy, but you can copy/paste this and run it on 
your machine.

There are two alternatives for fixing this problem:

1. If the current behavior is "as designed", update the docs to say that a nil 
slice will be returned of the decompressed output has length 0 and the dst 
input is nil.
2. Add a nil check and allocate a zero-length byte slice to handle this case.

I've attached patches for each of these alternatives (but don't apply them both 
since the two solutions are mutually exclusive).

Original issue reported on code.google.com by [email protected] on 28 Aug 2014 at 5:25

Attachments:

@GoogleCodeExporter
Copy link
Author

The documentation is actually correct as written. OP has paid attention to the 
wrong part.

"The returned slice may be a sub-slice of dst if dst was large enough to hold 
the entire encoded block."

In this case the encoded block has length 0. And dst (a nil slice) is large 
enough to hold zero elements. So a sub-slice of dst, dst[:0], is returned.

A subslice of a nil slice is nil itself. http://play.golang.org/p/HR9EsRYhJo

The statement in question by OP is the lower precedence "Otherwise" clause 
which applies only if the first one quoted here does not apply.

Original comment by [email protected] on 5 Nov 2014 at 11:08

@GoogleCodeExporter
Copy link
Author

OP here. I suppose you're right, the nil slice is a subslice of the nil slice 
which is big enough to contain 0 bytes. So the docs are correct.

Original comment by [email protected] on 5 Nov 2014 at 11:17

@GoogleCodeExporter
Copy link
Author

Oops. I copied the docs from Encode().

Decode() has similar docs, you just have to replace "encoded block" in my 
previous response with "decoded block".

Sorry for any confusion that caused.

Original comment by [email protected] on 5 Nov 2014 at 11:19

@GoogleCodeExporter
Copy link
Author

FWIW, because of things like this you almost never want to check if a slice is 
nil in Go. Instead it is generally better to check that its length is 0.

Original comment by [email protected] on 5 Nov 2014 at 11:25

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

No branches or pull requests

1 participant