code review 4958041: snappy: initial check-in of package snappy.

snappy: initial check-in of package snappy.

[nigeltao, 2011-08-25 03:39:53]

Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/snappy-go/

[adg, 2011-08-25 03:55:25]

http://codereview.appspot.com/4958041/diff/3/snappy/decode.go
File snappy/decode.go (right):

http://codereview.appspot.com/4958041/diff/3/snappy/decode.go#newcode13
snappy/decode.go:13: // ErrCorrupt reports that the input is not a valid snappy
block.
The word "Invalid" seems more precise, but this is just bike shedding.

[bradfitz, 2011-08-30 22:42:34]

LGTM

I didn't check for instance that it matches the C++ version, but the Go looks
fine.

It would be nice to have a testdata/file-made-by-cpp-snappy that you decompress
to double-check that you get the same thing.

And perhaps something similar with encoding.

http://codereview.appspot.com/4958041/diff/3/snappy/decode.go
File snappy/decode.go (right):

http://codereview.appspot.com/4958041/diff/3/snappy/decode.go#newcode24
snappy/decode.go:24: func decodedLen(src []byte) (int, int, os.Error) {
whenever I see two identical return types in a row, I tend to prefer them named.

private function, though, so I don't care too much.

http://codereview.appspot.com/4958041/diff/3/snappy/decode.go#newcode88
snappy/decode.go:88: copy(dst[d:], src[s:s+length])
I see this is safe to ignore the return value of copy from the previous checks
on line 85, but I wonder if it wouldn't be just as efficient and more warm &
cozy to remove those checks and move them down here, based on the return value
of copy instead.

I again don't really care, though.  It's fine as-is.

http://codereview.appspot.com/4958041/diff/3/snappy/snappy_test.go
File snappy/snappy_test.go (right):

http://codereview.appspot.com/4958041/diff/3/snappy/snappy_test.go#newcode36
snappy/snappy_test.go:36: t.Fatal(err)
include context in error message?  maybe:

t.Fatalf("i=%d: %v", i, err)

[nigeltao, 2011-08-31 11:59:21]

*** Submitted as fa2afaefa608 ***

snappy: initial check-in of package snappy.

R=bradfitz, adg
CC=golang-dev
http://codereview.appspot.com/4958041