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

Add string and bytes buffer convert trick in README #1151

Merged
merged 2 commits into from Nov 10, 2021

Conversation

ichxxx
Copy link
Contributor

@ichxxx ichxxx commented Nov 9, 2021

close #1031

README.md Outdated
@@ -485,6 +485,28 @@ statusCode, body, err := fasthttp.Get(nil, "http://google.com/")
uintBuf := fasthttp.AppendUint(nil, 1234)
```

* String and `[]byte` buffer may converts wihtout memory allocation
Copy link
Collaborator

@erikdubbelboer erikdubbelboer Nov 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* String and `[]byte` buffer may converts wihtout memory allocation
* String and `[]byte` buffers may converted without memory allocations

README.md Outdated
The underlying structure of `[]byte` buffer has only one `Len` field more than String.
So, we can construct one directly from the other.
Copy link
Collaborator

@erikdubbelboer erikdubbelboer Nov 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's useful to mention here.

Copy link
Contributor Author

@ichxxx ichxxx Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some people are wondering how it works, but if people want to know more details, they'll Google it. It's ok to delete.

README.md Outdated
This is an unsafe way,
please make sure not to modify the bytes in the `[]byte` buffer if the result string still survives.
Copy link
Collaborator

@erikdubbelboer erikdubbelboer Nov 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see this warning bigger and bold as it's really important! It's not just a note.

Copy link
Contributor Author

@ichxxx ichxxx Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@erikdubbelboer erikdubbelboer merged commit c078a9d into valyala:master Nov 10, 2021
@erikdubbelboer
Copy link
Collaborator

erikdubbelboer commented Nov 10, 2021

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants