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

b2s and s2b #1186

Closed
stokito opened this issue Jan 6, 2022 · 3 comments
Closed

b2s and s2b #1186

stokito opened this issue Jan 6, 2022 · 3 comments

Comments

@stokito
Copy link
Contributor

stokito commented Jan 6, 2022

The b2s (bytes to string) and s2b (string to byte sice) functions from bytesconv.go are named in lower case and can't be used from outside of fasthttp package. I do really need them too for two applications.

So I extracted them into a separate micro module https://github.com/stokito/go-strbytes
To make them usable I had to rename them to upper case S2b but that looks ugly and I gave more clear names.
But then I decided to check how the similar functions looks in other libraries and I was wondered that they are look different:

github.com/buger/jsonparser@v1.1.1/bytes_unsafe.go

// A hack until issue golang/go#2632 is fixed.
// See: https://github.com/golang/go/issues/2632
func bytesToString(b *[]byte) string {
	return *(*string)(unsafe.Pointer(b))
}

func StringToBytes(s string) []byte {
	b := make([]byte, 0, 0)
	bh := (*reflect.SliceHeader)(unsafe.Pointer(&b))
	sh := (*reflect.StringHeader)(unsafe.Pointer(&s))
	bh.Data = sh.Data
	bh.Cap = sh.Len
	bh.Len = sh.Len
	runtime.KeepAlive(s)
	return b
}

Here :

  • The bytesToString receives a pointer which looks like a hack for old Golang. Or maybe this is still valid thing to do?
  • The StringToBytes has an additional line b := make([]byte, 0, 0) which is absent in the fasthttp's s2b. Maybe this is also needed?

Now the similar function from github.com/mailru/easyjson@v0.7.7/jlexer/bytestostr.go

// bytesToStr creates a string pointing at the slice to avoid copying.
//
// Warning: the string returned by the function should be used with care, as the whole input data
// chunk may be either blocked from being freed by GC because of a single string or the buffer.Data
// may be garbage-collected even when the string exists.
func bytesToStr(data []byte) string {
	h := (*reflect.SliceHeader)(unsafe.Pointer(&data))
	shdr := reflect.StringHeader{Data: h.Data, Len: h.Len}
	return *(*string)(unsafe.Pointer(&shdr))
}

The function makes an additional steps that are missing in the b2s from fasthttp.

So my question is: could anybody tell exactly the correct implementation of those functions?
Or maybe we must send a PR to that libraries.

@erikdubbelboer
Copy link
Collaborator

We don't expose these methods because they are very dangerous to use. If people have to copy them into their code they can see how it works and are hopefully more aware of the dangers.

The pointer for the argument to bytesToString doesn't really do anything. Just makes calling the method more complicated.

The b := make([]byte, 0, 0) is useless as well, you don't need an empty slice to start with, a nil slice is just as fine. The runtime.KeepAlive(s) does nothing, see #1107

The bytesToStr that uses a reflect.StringHeader isn't needed either unless some future version of Go changes either reflect.SliceHeader or reflect.StringHeader so that they don't overlap anymore. In theory this is safer but in practice I don't see that ever changing.

So in conclusion all these methods are basically the same, you can copy either, but the fasthttp ones are the simplest I guess.

@erikdubbelboer
Copy link
Collaborator

I did just realize that the easyjson implementation is wrong: mailru/easyjson#358

@stokito
Copy link
Contributor Author

stokito commented Jan 17, 2022

@erikdubbelboer thank you for the finding, great job 👍

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

No branches or pull requests

2 participants