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

[GoogleCodeExporter]

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 Dave.Revell@gmail.com on 28 Aug 2014 at 5:25

Attachments:

• alloc_zero_length_return.diff
• doc_as_designed.diff

[GoogleCodeExporter]

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 bmat...@janrain.com on 5 Nov 2014 at 11:08

[GoogleCodeExporter]

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 Dave.Revell@gmail.com on 5 Nov 2014 at 11:17

[GoogleCodeExporter]

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 bmat...@janrain.com on 5 Nov 2014 at 11:19

[GoogleCodeExporter]

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 bmat...@janrain.com on 5 Nov 2014 at 11:25

[nigeltao]

Yes, I think this is working as intended: a nil slice is a sub-slice of a nil slice.

Sorry for the late reply.