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

bytes: document NewBuffer takes ownership of provided slice #19383

Closed
zwass opened this issue Mar 3, 2017 · 2 comments
Closed

bytes: document NewBuffer takes ownership of provided slice #19383

zwass opened this issue Mar 3, 2017 · 2 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@zwass
Copy link

zwass commented Mar 3, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.8 darwin/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/zwass/dev/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.8/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.8/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/4k/2q3ctn7d5_xb03_zsb3v6lgr0000gn/T/go-build693792700=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

https://play.golang.org/p/wW5LCvEMT5

What did you expect to see?

inJSON is unchanged. outJSON contains the original contents of inJSON.

What did you see instead?

Contents of inJSON have been replaced by gzip binary garbage. Contents of outJSON include the gzip binary header.

I think the biggest issue here is that the bytes.NewBuffer documentation does not make it clear that the provided []byte will actually be used for the backing storage of the buffer.

NewBuffer creates and initializes a new Buffer using buf as its initial contents. It is intended to prepare a Buffer to read existing data.

This would lead the user to believe that the data will be copied into the buffer as it is with bytes.NewBufferString, when in fact writes to the buffer may alter that underlying data.

My other question is whether it is expected that the gzip writer would not function properly (by overwriting the data with the header before compressing the first bit of data) when reading and writing the same []byte? I suspect that this is a strange edge case that should not be used and is not supported.

@bradfitz bradfitz changed the title Unexpected behavior with bytes.Buffer and gzip.Writer bytes: document NewBuffer takes ownership of provided slice Mar 3, 2017
@bradfitz bradfitz added Documentation NeedsFix The path to resolution is known, but the work has not been done. labels Mar 3, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Mar 3, 2017
@ALTree
Copy link
Member

ALTree commented Jun 2, 2017

I sent CL 44691 to fix the NewBuffer doc.

Regarding this:

is expected that the gzip writer would not function properly (by overwriting the data with the header before compressing the first bit of data) when reading and writing the same []byte?

I'd say: yes, this is expected and not very surprising, and I don't think we need to explicitly say that in the documentation.

@gopherbot
Copy link

CL https://golang.org/cl/44691 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants