Need to be able to pass options to Dir::open()

[SpamapS]

The assumed O_PATH|O_CLOEXEC is not sufficient in many cases. They can be the default but there needs to be some way to pass them in.

[tailhook]

Sure, would you like to make a PR?

I'm not sure about exact API, though. We may discuss it here before doing work.

[SpamapS]

I've been poking at it. Simplest thing is to add an open_flags fn that takes a c_int and then plumb it through via _open. However, I wonder if the whole of this crate should be moved into the 'nix' crate, where there are some nice helpers for dealing with constants like O_PATH, and where it would really fit rather well.

[tailhook]

Well you may ask in nix's issues, I'm not sure.

open_flags sounds fine, but I'm not sure about integer flags. On the other hand, making our own wrapper around flags isn't great idea. But the issue is wider: open_file and sub_dir should have _flags variant too, right?

Can you desribe your use case a little bit further? I'm not sure which flags make sense for directories.

[SpamapS]

I'm porting this to Rust: https://github.com/projectatomic/bubblewrap here's my repo https://github.com/SpamapS/rubble .. it uses openat() quite a bit, but not with O_PATH|O_CLOEXEC. I agree, I think O_RDONLY might be implied with dirs, but I'm trying to do an exact port.

[tailhook]

Well, firstly for the cases where O_CLOEXEC is not desired it's better to explicitly remove the flag just before exec (after fork). This ensures that file descriptor is not mistakenly leaked (this is what unshare library does by the way, do you plan using unshare and libmount? I'm interested in other use cases for those libraries).

The case for O_PATH is just for Dir. I.e. this is a thing that encapsulates the path file descriptor. I.e. if you want to open a file you use Dir::open("/base").open_file("path/to/file"). While it's possible to open FD without O_PATH and use it as a dirfd in openat() syscall, I don't think there are lots of use cases for that.

For O_RDONLY, here is the excerpt from the docs:

When O_PATH is specified in flags, flag bits other than O_CLOEXEC, O_DIRECTORY, and O_NOFOLLOW are ignored.

---

Looking at the code of bubblewrap (looking at openat call only), there are 4 cases:

./bubblewrap.c:651:  dir_fd = openat (proc_fd, dir, O_RDONLY | O_PATH);
./utils.c:303:  dfd = openat (proc_fd, "self/fd", O_DIRECTORY | O_RDONLY | O_NONBLOCK | O_CLOEXEC | O_NOCTTY);
./utils.c:385:  fd = openat (dirfd, path, O_RDWR | O_CLOEXEC, 0);
./utils.c:567:  fd = openat (dirfd, path, O_CLOEXEC | O_RDONLY);

1. O_PATH + O_RDONLY is error (O_RDONLY is useless)
2. Omitting O_CLOEXEC is oversight
3. O_DIRECTORY | O_RDONLY | O_NONBLOCK | O_CLOEXEC | O_NOCTTY -- I think O_DIRECTORY implies O_NOCTTY, and makes O_NONBLOCK useless. So this is just dir.sub_dir(...)

[SpamapS]

Hey thanks for taking a deep dive into it with me. I think it might be worth reporting these as issues in bubblewrap. I'll go ahead and do that, and just use your openat lib as-is. 👍

[tailhook]

Seems that options aren't that useful. I mean it's fine to add more options, but we should keep reasonably safe and expressive API, and that in turns means we should design the API with use cases in mind. So feel free to open a similar issue describing more specific use cases.