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

proposal: cmd/cgo: change GoStringN, GoBytes to take C.size_t #61361

Closed
bemasc opened this issue Jul 14, 2023 · 4 comments
Closed

proposal: cmd/cgo: change GoStringN, GoBytes to take C.size_t #61361

bemasc opened this issue Jul 14, 2023 · 4 comments
Labels
Milestone

Comments

@bemasc
Copy link
Contributor

bemasc commented Jul 14, 2023

Proposal

Change the signature of GoStringN(*C.char, C.int) to GoStringN(*C.char, C.size_t), and similarly for GoBytes. This would enable C->Go transfer of >2GB strings on most Go systems, and avoid a cast when wrapping C APIs that follow modern usage guidelines.

This would also improve consistency with _GoStringLen(), which already returns size_t.

Costs

All existing uses of GoStringN() would require a change (either removing or adding a typecast).

@gopherbot gopherbot added this to the Proposal milestone Jul 14, 2023
@ianlancetaylor
Copy link
Contributor

That proposed change would break the spirit of the Go 1 compatibility guarantee, so we're not going to do it. (The Go 1 compatibility guarantee doesn't cover cgo, but, we're still not going to gratuitously break existing code without a really good reason.) And though the issue is marked "Go 2", there isn't going to be a Go 2, and we aren't going to make a cgov2 for something like this.

So I suggest a different proposal:

// GoStringBigN is like GoStringN but for very large strings.
func GoStringBigN(*C.char, C.size_t)

// GoBytesBig is like GoBytes but for very large data.
func GoBytesBig(unsafe.Pointer, C.size_t)

@bemasc
Copy link
Contributor Author

bemasc commented Jul 14, 2023

That would be fine with me. Alternatively, perhaps it would be possible to widen the argument types to accept both (e.g., func GoStringN[T C.int | C.size_t](*C.char, T) in generics style).

@bemasc bemasc changed the title proposal: Go 2: Use C.size_t in C.GoStringN() and C.GoBytes() proposal: Allow C.size_t in C.GoStringN() and C.GoBytes() Jul 14, 2023
@bcmills
Copy link
Contributor

bcmills commented Jul 17, 2023

I think we already provide too many cgo helper functions, and I would prefer that we deprecate them rather than expanding them.

In particular, these functions seem redundant with the recently-added unsafe.String, unsafe.Slice, and bytes.Clone. Consider:

func goStringN(p *C.char, n C.size_t) string {
	return string(unsafe.Slice((*byte)(unsafe.Pointer(p)), n))
}

func goBytes(p unsafe.Pointer, n C.size_t) []byte {
	return bytes.Clone(unsafe.Slice((*byte)(p), n))
}

@rsc rsc changed the title proposal: Allow C.size_t in C.GoStringN() and C.GoBytes() proposal: cmd/cgo: change GoStringN, GoBytes to take C.size_t Dec 4, 2023
@rsc
Copy link
Contributor

rsc commented Dec 4, 2023

This proposal has been declined as infeasible.
— rsc for the proposal review group

@rsc rsc closed this as completed Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Declined
Development

No branches or pull requests

5 participants