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: Linux: utimensat syscall takes a NULL path, wrapper can't #49699

Open
unbrice opened this issue Nov 20, 2021 · 3 comments
Open

x/sys/unix: Linux: utimensat syscall takes a NULL path, wrapper can't #49699

unbrice opened this issue Nov 20, 2021 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@unbrice
Copy link

unbrice commented Nov 20, 2021

What version of Go are you using (go version)?

x/sys package v0.0.0-20211117180635-dee7805ff2e1

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
The version of Golang we use at Google (I believe this is not public information, please correct me if I'm wrong and I'll share go env)

What did you do?

I opened the documentation of sys/unix and also read its source code.

What did you expect to see?

I expected to be able to do the equivalent to the futimens glibc function.
This could have been its own function. Alternatively the wrapper of utimensat could have given empty strings the special meaning that NULL pointers have for the syscall.

(On Linux, in the glibc, futimens is implemented on top of utimensat by passing a NULL pointer as path. This is described in the NOTES section of the manpage.)

I also expected to see Futimes implemented in a way that did not go through /proc

What did you see instead?

  • The wrapper of utimensat, named UtimesNanoAt, takes a string for the path and does not have a special case to pass NULL when the string is empty.
  • The wrapper of Utimes goes to /proc instead of taking advantage of a NULL path
  • There is no equivalent for futimens

Please let me know if you'd like me to contribute a fix. Unless someone suggest another approach, I offer to send a PR that leaves UtimesNanoAt unchanged and adds FUtimesNano.

@gopherbot gopherbot added this to the Unreleased milestone Nov 20, 2021
@unbrice unbrice changed the title x/sys: Linux: the utimensat syscan take a NULL path, its wrapper can't x/sys: Linux: utimensat syscall takes a NULL path, wrapper can't Nov 20, 2021
@ianlancetaylor
Copy link
Contributor

When I read the man page of the utimensat I don't see any difference between a NULL pathname argument as opposed to a pathname argument that is the empty string. When is it important to pass NULL, rather than a pointer to a NUL byte, to utimensat?

@ianlancetaylor ianlancetaylor changed the title x/sys: Linux: utimensat syscall takes a NULL path, wrapper can't x/sys/unix: Linux: utimensat syscall takes a NULL path, wrapper can't Nov 22, 2021
@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Nov 22, 2021
@unbrice
Copy link
Author

unbrice commented Nov 22, 2021

As per the NOTES section of the manpage, the difference is as follow;

  • a NULL path results in the call being applied to the file referred to by the file descriptor dirfd (which does not need to be a directory). This allows implementing futimens/FUtimesNano.
  • an empty path returns the ENOENT error.

@ianlancetaylor
Copy link
Contributor

Thanks. I guess we should consider adding a new syscall wrapper that passes NULL. Perhaps this should be Futimens.

@ianlancetaylor ianlancetaylor removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants