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: Buffer.Bytes() is safe with Read, Reset and Truncate #18052

Closed
mvdan opened this issue Nov 25, 2016 · 4 comments
Closed

bytes: Buffer.Bytes() is safe with Read, Reset and Truncate #18052

mvdan opened this issue Nov 25, 2016 · 4 comments

Comments

@mvdan
Copy link
Member

mvdan commented Nov 25, 2016

#13671 was filed to add a note in the documentation that the Buffer.Bytes() slice may become invalid if the buffer is later reused. The docs now look like:

// Bytes returns a slice of length b.Len() holding the unread portion of the buffer.
// The slice is valid for use only until the next buffer modification (that is,
// only until the next call to a method like Read, Write, Reset, or Truncate).
// The slice aliases the buffer content at least until the next buffer modification,
// so immediate changes to the slice will affect the result of future reads.

I think this is inaccurate. As far as I can tell, Read, Reset and Truncate are all safe - their use alone never changes the contents of the buffer.

There are two ways I can see the data becoming invalid:

  • a Bytes() call modifies the slice contents directly (already mentioned in the last sentence)
  • Truncate() is called and any write happens (writes without truncation can't harm)

So I think the doc should reflect that. Perhaps something like:

// The slice is valid for use only until the next buffer modification (that is,
// only until the next call to a method like Truncate or Reset and any writes).
@mvdan
Copy link
Member Author

mvdan commented Nov 25, 2016

To clarify, I wanted to open an issue first before sending a CL since I might be wrong about this.

@rakyll rakyll added this to the Go1.9 milestone Nov 25, 2016
@mvdan
Copy link
Member Author

mvdan commented Dec 5, 2016

Since noone has pointed out any obvious mistakes in my reasoning above, I'm sending a CL for 1.9.

@gopherbot
Copy link

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

@mvdan
Copy link
Member Author

mvdan commented Dec 5, 2016

Closing as per Brad's comments on the CL.

@mvdan mvdan closed this as completed Dec 5, 2016
@golang golang locked and limited conversation to collaborators Dec 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants