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: export Fcntl and make more POSIX compliant #24677
Comments
Good news: Problem: git-codereview: cannot amend change: multiple changes pending: So, how should I proceed? I am trying to follow "Multiple-Commit Work Branches" paragraph in the git-codereview docs, but it seems to be missing a few steps. ?? |
PS: just so I dont forget, I'm noting here that if my code changes are accepted, there will need to be a robo-auto-update, to fix the autogenerated zsyscall_xxx files (since I was informed that may only happen on the official build farm) |
(Pasting from my reply in https://golang.org/cl/104255) After reading https://golang.org/pkg/unsafe/#Pointer and checking on the discussion around the introduction of the Ioctl* functions (which are quite similar in the way they work), I think we shouldn't add a raw exported Fcntl at all. Regardless of whether arg is int or uintptr, users would have to use unsafe.Pointer if they want to pass pointers. E.g. see the discussion in https://golang.org/cl/44009 and #12574, especially Ian's comments #12574 (comment) and #12574 (comment). Thus, I'd propose to add Fcntl* functions for each specific type (we already have FcntlFlock anyway) which will use the currently existing fcntl function. We might want to change its implementation to use uintptr arg instead of int. This would allow users to use these functions in a type-safe manner without using the unsafe package. @bradfitz, @ianlancetaylor what do you think? |
Agree. Specific type-safe fnctl sounds good. |
Change https://golang.org/cl/104736 mentions this issue: |
go version go1.10 linux/amd64
This is related to issue #24649 and thus to code review
https://go-review.googlesource.com/c/sys/+/104255
I am going to be attaching a new code PR to this issue. If accepted, I can then cancel the prior one.
It was discussed in 104255, how the potentially most clean approach is to finally export fcntl as Fcntl.
Unfortunately, the existing x/sys/unix.fcntl is not standards compliant with its type.
The third argument is declared as "int": however, sometimes, fcntl calls require the third argument to be a pointer.
This mandates that the third argument be changed to uintptr, rather than int.
Happily, it is currently an internal function only, so making the change does not require too many cascading updates.
The text was updated successfully, but these errors were encountered: