Skip to content

syscall: add/use faccessat2 on Linux #53695

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.

Sign up for GitHub

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

Jump to bottom
Closed
kolyshkin opened this issue Jul 5, 2022 · 5 comments
Closed

syscall: add/use faccessat2 on Linux #53695

kolyshkin opened this issue Jul 5, 2022 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@kolyshkin
Copy link
Contributor

Linux kernel 5.8 added the faccessat2 syscall taking a flags argument.
Attempt to use it in Faccessat and fall back to the existing
implementation mimicking glibc faccessat.

This is similar to [1] amended by [2]. Required for [3].

[1] https://go-review.googlesource.com/c/sys/+/246537
[2] https://go-review.googlesource.com/c/sys/+/246817
[3] https://go-review.googlesource.com/c/go/+/414824
@gopherbot gopherbot added this to the Proposal milestone Jul 5, 2022
@ianlancetaylor
Copy link
Member

Changing syscall.Faccessat to use the faccessat2 system call is not an API change and does not require a proposal.

Are you suggesting an API change? In general the syscall package is frozen and new API should not be added to it.

@ianlancetaylor ianlancetaylor added NeedsFix The path to resolution is known, but the work has not been done. and removed Proposal labels Jul 6, 2022
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Backlog Jul 6, 2022
@ianlancetaylor ianlancetaylor changed the title proposal: syscall: add/use faccessat2 on Linux syscall: add/use faccessat2 on Linux Jul 6, 2022
@kolyshkin
Copy link
Contributor Author

Changing syscall.Faccessat to use the faccessat2 system call is not an API change and does not require a proposal.

According to checks performed by src/cmd/api, adding a SYS_FACCESSAT2 constant is.

In such cases, api/README requires to to have a file named api/next/NNN.txt, where NNN is the issue number proposing a change -- so I had to open this issue to have a value for NNN in https://go-review.googlesource.com/c/go/+/416115

Are you suggesting an API change? In general the syscall package is frozen and new API should not be added to it.

I believe I do not (except for adding a new const).

@ianlancetaylor
Copy link
Member

Fair point about SYS_FACCESSAT2 requiring a proposal. But looking at the current CL, I don't think we need to export that constant at all.

@kolyshkin
Copy link
Contributor Author

Fair point about SYS_FACCESSAT2 requiring a proposal. But looking at the current CL, I don't think we need to export that constant at all.

I thought about making it _SYS_FACCESSAT2 at first, but AFAICS the syscall code generator expects SYS_ prefix.

@kolyshkin
Copy link
Contributor Author

OK, redone without API change as suggested by @tklauser, so we don't change the API and thus do not need this issue.

@golang golang locked and limited conversation to collaborators Jul 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@ianlancetaylor @kolyshkin @gopherbot