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

x/sys/unix: Add wrapper for readv/writev #36201

Closed
Merovius opened this issue Dec 18, 2019 · 7 comments
Closed

x/sys/unix: Add wrapper for readv/writev #36201

Merovius opened this issue Dec 18, 2019 · 7 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@Merovius
Copy link
Contributor

Merovius commented Dec 18, 2019

I'd like to propose exposing readv/writev (and maybe preadv/pwritev and maybeer preadv2/pwritev2) in x/sys/unix, as {Read,Write}v(fd int, iov [][]byte) (int, error).

My usecase is writing to a file in O_APPEND mode, which offers atomicity-guarantees per write. Issuing multiple writes loses those guarantees, so I have to resort to a) allocating a temporary buffer and concatenating the individual []bytes, or b) use unix.Syscall. The latter is fine, but a wrapper would allow to avoid unsafe.

I don't personally care about the p… and …v2 variants, but they could be added at the same time. I'm not sure how portable they are, though, beyond Linux.

I can send a CL if this is accepted.

@gopherbot gopherbot added this to the Unreleased milestone Dec 18, 2019
@Merovius
Copy link
Contributor Author

Merovius commented Dec 18, 2019

(See also #21676 which discusses, among other things, exposing an interface to writev for *os.File)

@ianlancetaylor
Copy link
Contributor

In general we permit adding any existing system call to x/net/unix. Using [][]byte as the second argument seems right to me. So this is fine.

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Dec 19, 2019
@tklauser tklauser changed the title x/net/unix: Add wrapper for readv/writev x/sys/unix: Add wrapper for readv/writev Dec 19, 2019
@tklauser
Copy link
Member

I assume you meant x/sys/unix, not x/net/unix?

It's fine to add the non-portable preadv/pwritev/preadv2/pwritev2 variants for Linux only. We already have several wrappers for Linux-only syscalls.

@Merovius
Copy link
Contributor Author

Merovius commented Dec 19, 2019

@tklauser yes I do, thanks, corrected my initial post as well :) I'll send a CL later today.

@gopherbot
Copy link

Change https://golang.org/cl/212157 mentions this issue: unix: add Readv/Writev family of syscalls for linux

@Merovius
Copy link
Contributor Author

I only have Linux machines, so AIUI I can't generate the constants and everything for other platforms.
Also, I mostly copied the interaction with the race-detector from Read/Write - I have no idea if that was needed or correct in this case.

MosakujiHokuto pushed a commit to MosakujiHokuto/sys that referenced this issue Mar 20, 2020
iovec read/writes (readv, writev, preadv, pwritev) are present on illumos,
but they weren't added to the package. This commit added those missing
features.

Updates golang/go#36201
@gopherbot
Copy link

Change https://golang.org/cl/224238 mentions this issue: unix: Add readv/writev for illumos

gopherbot pushed a commit to golang/sys that referenced this issue Mar 21, 2020
iovec read/writes (readv, writev, preadv, pwritev) are present on
illumos, but they weren't added to the package. This commit adds
those missing features.

Updates golang/go#36201

Change-Id: Iecf2bab7ef846f5ca5d693e833491d819618c15d
GitHub-Last-Rev: 81a479f
GitHub-Pull-Request: #64
Reviewed-on: https://go-review.googlesource.com/c/sys/+/224238
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
@golang golang locked and limited conversation to collaborators Mar 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted 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