add support for openat, fstatat, readlink, readlinkat #552
Conversation
Mic92
force-pushed
the
fstatat
branch
9 times, most recently
from
03c6d29
to
02e9d39
Compare
|
I cannot explain the build error happen on travis. Works fine locally. |
src/sys/stat.rs
Outdated
| mod consts { | ||
| use libc::c_int; | ||
|
|
||
| bitflags!( |
There was a problem hiding this comment.
Please use the constants from the libc crate. There's a macro libc_bitflags to help with this; for an example, see:
Lines 9 to 20 in 5c90289
It looks like you might have to add AT_NO_AUTOMOUNT to libc, as well as add the constants to non-linux systems. Let me know if you need any help or guidance doing that!
src/sys/stat.rs
Outdated
| use libc::c_int; | ||
|
|
||
| bitflags!( | ||
| flags FstatatFlag: c_int { |
There was a problem hiding this comment.
This should be called AtFlags according to the conventions: https://github.com/nix-rust/nix/blob/master/CONVENTIONS.md#bitflags
The flag is used in a bunch of other at functions that use these flags, like openat, linkat, unlinkat, ... So naming this after fstatat doesn't make too much sense. :-)
There was a problem hiding this comment.
Hm, not all of them use all these flags, but at least linkat does.
There was a problem hiding this comment.
I saw that nix unify some flags with the same name prefix, but I did not understand the reasoning here. Because some flags are only accepted by some system calls. For instance AT_EMPTY_PATH is not accepted by fchownat and even for the same function different operating systems differ in flags they accept for a system call wrapper. Would this not lead to subtle bugs, when porting applications?
|
Oh I'm also confused about the CI failures! Tests pass on my machine. I probably won't get to look into it in detail for a few more days. If you figure out out, please let me know what it was! |
|
wait for rust-lang/libc#549 to be merged. |
Mic92
changed the title
fstatat
Mic92
force-pushed
the
fstatat
branch
2 times, most recently
from
173223c
to
de49221
Compare
Mic92
force-pushed
the
fstatat
branch
2 times, most recently
from
e2ca81a
to
4bd1af0
Compare
src/fcntl.rs
Outdated
| unsafe { libc::readlink(cstr.as_ptr(), buffer.as_mut_ptr() as *mut c_char, buffer.len() as size_t) } | ||
| })); | ||
|
|
||
| Errno::result(res).map(move |len| OsStr::from_bytes(&buffer[..(len as usize)])) |
There was a problem hiding this comment.
ok, this solution is wrong: if the buffer is too small, the buffer is truncated without notifications. I see two options here:
- Remove buffer from arguments and reallocate buffer internally until it fits the result of readlink (this is what the rust/go stdlib do)
- return ENAMETOOLONG if len > buffer.len()
Which one fits into the philosophy of nix better?
There was a problem hiding this comment.
I decided to use strategy 2 for the moment. If people want to use the first option, they could also use the rust std.
|
Looks good, thanks! @homu r+ |
|
📌 Commit 55d7b5c has been approved by |
|
⚡ Test exempted - status |
add support for openat, fstatat, readlink, readlinkat
No description provided.