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.

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

pkg/lockfile: nits #1336

Merged
merged 3 commits into from
Sep 12, 2022
Merged

pkg/lockfile: nits #1336

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2076605
pkg/lockfile: fix openLock doc
kolyshkin Sep 8, 2022
463d50f
pkg/lockfile: fix openLock file permissions
kolyshkin Sep 8, 2022
7d52f17
pkg/lockfile: wrap error in openLock
kolyshkin Sep 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 9 additions & 11 deletions pkg/lockfile/lockfile_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,17 @@ func newLastWriterID() []byte {
}

// openLock opens the file at path and returns the corresponding file
// descriptor. Note that the path is opened read-only when ro is set. If ro
// is unset, openLock will open the path read-write and create the file if
// necessary.
// descriptor. The path is opened either read-only or read-write,
// depending on the value of ro argument.
//
// openLock will create the file and its parent directories,
// if necessary.
func openLock(path string, ro bool) (fd int, err error) {
if ro {
fd, err = unix.Open(path, os.O_RDONLY|unix.O_CLOEXEC|os.O_CREATE, 0)
} else {
fd, err = unix.Open(path,
os.O_RDWR|unix.O_CLOEXEC|os.O_CREATE,
unix.S_IRUSR|unix.S_IWUSR|unix.S_IRGRP|unix.S_IROTH,
)
flags := os.O_RDONLY | unix.O_CLOEXEC | os.O_CREATE
if !ro {
flags |= os.O_RDWR
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This relies on the fact that O_RDONLY value is 0.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s

  • Extremely non-obvious to careless users.
  • Not guaranteed to be true, and in fact not true on at least one Go-supported platform right now (though, to be fair, not a platform currently supported by Podman).

Please just do the obviously-correct thing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1343

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More generally, the assumption is O_RDONLY does not have any bits set that O_RDWR doesn't.

That is true even for zOS (the only Golang platform where O_RDONLY != 0). In zOS, O_RDONLY is 2 and O_RDWR is 3.

Having said that, your code is definitely better as it does not have any such undocumented assumptions.

}

fd, err = unix.Open(path, flags, 0o644)
if err == nil {
return
}
Expand Down