Skip to content

Commit 76c657f

Browse files
sunfishcodesunfishcode
authored
Make fadvise's len parameter an Option<NonZeroU64>. (#1335)
This reflects how zero is special-cased to mean the length to the end of the file. Fixes #1333.
1 parent cc673a1 commit 76c657f

8 files changed

Lines changed: 119 additions & 33 deletions

File tree

‎CHANGES.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,5 +223,12 @@ specific socket types using `From`/`Into`/`TryFrom`/`TryInto` conversions.
223223

224224
[`SocketAddrAny`]: https://docs.rs/rustix/1.0.0/rustix/net/struct.SocketAddrAny.html
225225

226+
The `len` parameter to [`rustix::fs::fadvise`] has changed from `u64` to
227+
`Option<NonZeroU64>`, to reflect that zero is a special case meaning the
228+
advice applies to the end of the file. To convert an arbitrary `u64` value to
229+
`Option<NonZeroU64>`, use `NonZeroU64::new`.
230+
231+
[`rustix::fs::fadvise`]: https://docs.rs/rustix/1.0.0/rustix/fs/fn.fadvise.html
232+
226233
All explicitly deprecated functions and types have been removed. Their
227234
deprecation messages will have identified alternatives.

‎src/backend/libc/fs/syscalls.rs‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,6 @@ use crate::ffi::CString;
1010
use crate::ffi::{self, CStr};
1111
#[cfg(not(any(target_os = "espidf", target_os = "vita")))]
1212
use crate::fs::Access;
13-
#[cfg(not(any(
14-
apple,
15-
netbsdlike,
16-
target_os = "solaris",
17-
target_os = "dragonfly",
18-
target_os = "espidf",
19-
target_os = "haiku",
20-
target_os = "redox",
21-
target_os = "vita",
22-
)))]
23-
use crate::fs::Advice;
2413
#[cfg(not(any(target_os = "espidf", target_os = "redox")))]
2514
use crate::fs::AtFlags;
2615
#[cfg(not(any(
@@ -75,6 +64,17 @@ use {
7564
crate::backend::conv::nonnegative_ret,
7665
crate::fs::{copyfile_state_t, CloneFlags, CopyfileFlags},
7766
};
67+
#[cfg(not(any(
68+
apple,
69+
netbsdlike,
70+
target_os = "solaris",
71+
target_os = "dragonfly",
72+
target_os = "espidf",
73+
target_os = "haiku",
74+
target_os = "redox",
75+
target_os = "vita",
76+
)))]
77+
use {crate::fs::Advice, core::num::NonZeroU64};
7878
#[cfg(any(apple, linux_kernel, target_os = "hurd"))]
7979
use {crate::fs::XattrFlags, core::mem::size_of, core::ptr::null_mut};
8080
#[cfg(linux_kernel)]
@@ -1213,9 +1213,17 @@ pub(crate) fn copy_file_range(
12131213
target_os = "redox",
12141214
target_os = "vita",
12151215
)))]
1216-
pub(crate) fn fadvise(fd: BorrowedFd<'_>, offset: u64, len: u64, advice: Advice) -> io::Result<()> {
1216+
pub(crate) fn fadvise(
1217+
fd: BorrowedFd<'_>,
1218+
offset: u64,
1219+
len: Option<NonZeroU64>,
1220+
advice: Advice,
1221+
) -> io::Result<()> {
12171222
let offset = offset as i64;
1218-
let len = len as i64;
1223+
let len = match len {
1224+
None => 0,
1225+
Some(len) => len.get() as i64,
1226+
};
12191227

12201228
// Our public API uses `u64` following the [Rust convention], but the
12211229
// underlying host APIs use a signed `off_t`. Converting these values may

‎src/backend/linux_raw/fs/syscalls.rs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ use crate::fs::{
4141
};
4242
use crate::io;
4343
use core::mem::MaybeUninit;
44+
use core::num::NonZeroU64;
4445
#[cfg(any(target_arch = "mips64", target_arch = "mips64r6"))]
4546
use linux_raw_sys::general::stat as linux_stat64;
4647
use linux_raw_sys::general::{
@@ -364,7 +365,17 @@ pub(crate) fn fallocate(
364365
}
365366

366367
#[inline]
367-
pub(crate) fn fadvise(fd: BorrowedFd<'_>, pos: u64, len: u64, advice: Advice) -> io::Result<()> {
368+
pub(crate) fn fadvise(
369+
fd: BorrowedFd<'_>,
370+
pos: u64,
371+
len: Option<NonZeroU64>,
372+
advice: Advice,
373+
) -> io::Result<()> {
374+
let len = match len {
375+
None => 0,
376+
Some(len) => len.get(),
377+
};
378+
368379
// On ARM, the arguments are reordered so that the `len` and `pos` argument
369380
// pairs are aligned. And ARM has a custom syscall code for this.
370381
#[cfg(target_arch = "arm")]

‎src/fs/fadvise.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
use crate::{backend, io};
22
use backend::fd::AsFd;
33
use backend::fs::types::Advice;
4+
use core::num::NonZeroU64;
45

56
/// `posix_fadvise(fd, offset, len, advice)`—Declares an expected access
67
/// pattern for a file.
78
///
9+
/// If `len` is `None`, the advice extends to the end of the file.
10+
///
811
/// # References
912
/// - [POSIX]
1013
/// - [Linux]
@@ -15,6 +18,11 @@ use backend::fs::types::Advice;
1518
/// [FreeBSD]: https://man.freebsd.org/cgi/man.cgi?query=posix_fadvise&sektion=2
1619
#[inline]
1720
#[doc(alias = "posix_fadvise")]
18-
pub fn fadvise<Fd: AsFd>(fd: Fd, offset: u64, len: u64, advice: Advice) -> io::Result<()> {
21+
pub fn fadvise<Fd: AsFd>(
22+
fd: Fd,
23+
offset: u64,
24+
len: Option<NonZeroU64>,
25+
advice: Advice,
26+
) -> io::Result<()> {
1927
backend::fs::syscalls::fadvise(fd.as_fd(), offset, len, advice)
2028
}

‎src/net/addr.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ mod tests {
155155
fn test_layouts() {
156156
assert_eq_size!(SocketAddrLen, c::socklen_t);
157157

158-
#[cfg(not(windows))]
158+
#[cfg(not(any(windows, target_os = "redox")))]
159159
assert_eq!(
160160
memoffset::span_of!(c::msghdr, msg_namelen).len(),
161161
size_of::<SocketAddrLen>()

‎tests/fs/file.rs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
#[cfg(not(any(
2+
apple,
3+
netbsdlike,
4+
target_os = "solaris",
5+
target_os = "dragonfly",
6+
target_os = "espidf",
7+
target_os = "haiku",
8+
target_os = "redox",
9+
)))]
10+
use core::num::NonZeroU64;
11+
112
#[cfg(not(target_os = "redox"))]
213
#[test]
314
fn test_file() {
@@ -88,9 +99,8 @@ fn test_file() {
8899
target_os = "dragonfly",
89100
target_os = "espidf",
90101
target_os = "haiku",
91-
target_os = "redox",
92102
)))]
93-
rustix::fs::fadvise(&file, 0, 10, rustix::fs::Advice::Normal).unwrap();
103+
rustix::fs::fadvise(&file, 0, NonZeroU64::new(10), rustix::fs::Advice::Normal).unwrap();
94104

95105
rustix::fs::fsync(&file).unwrap();
96106

@@ -99,7 +109,6 @@ fn test_file() {
99109
target_os = "dragonfly",
100110
target_os = "espidf",
101111
target_os = "haiku",
102-
target_os = "redox",
103112
)))]
104113
rustix::fs::fdatasync(&file).unwrap();
105114

@@ -134,7 +143,6 @@ fn test_file() {
134143
solarish,
135144
target_os = "haiku",
136145
target_os = "netbsd",
137-
target_os = "redox",
138146
target_os = "wasi",
139147
)))]
140148
{

‎tests/fs/invalid_offset.rs‎

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ fn invalid_offset_fallocate() {
6767
)))]
6868
#[test]
6969
fn invalid_offset_fadvise() {
70+
use core::num::NonZeroU64;
7071
use rustix::fs::{fadvise, openat, Advice, Mode, OFlags, CWD};
7172
let tmp = tempfile::tempdir().unwrap();
7273
let dir = openat(CWD, tmp.path(), OFlags::RDONLY, Mode::empty()).unwrap();
@@ -79,21 +80,63 @@ fn invalid_offset_fadvise() {
7980
.unwrap();
8081

8182
// `fadvise` never fails on invalid offsets.
82-
fadvise(&file, i64::MAX as u64, i64::MAX as u64, Advice::Normal).unwrap();
83-
fadvise(&file, u64::MAX, 0, Advice::Normal).unwrap();
84-
fadvise(&file, i64::MAX as u64, 1, Advice::Normal).unwrap();
85-
fadvise(&file, 1, i64::MAX as u64, Advice::Normal).unwrap();
86-
fadvise(&file, i64::MAX as u64 + 1, 0, Advice::Normal).unwrap();
87-
fadvise(&file, u64::MAX, i64::MAX as u64, Advice::Normal).unwrap();
83+
fadvise(
84+
&file,
85+
i64::MAX as u64,
86+
NonZeroU64::new(i64::MAX as u64),
87+
Advice::Normal,
88+
)
89+
.unwrap();
90+
fadvise(&file, u64::MAX, None, Advice::Normal).unwrap();
91+
fadvise(&file, i64::MAX as u64, NonZeroU64::new(1), Advice::Normal).unwrap();
92+
fadvise(&file, 1, NonZeroU64::new(i64::MAX as u64), Advice::Normal).unwrap();
93+
fadvise(&file, i64::MAX as u64 + 1, None, Advice::Normal).unwrap();
94+
fadvise(
95+
&file,
96+
u64::MAX,
97+
NonZeroU64::new(i64::MAX as u64),
98+
Advice::Normal,
99+
)
100+
.unwrap();
88101

89102
// `fadvise` fails on invalid lengths.
90-
fadvise(&file, u64::MAX, u64::MAX, Advice::Normal).unwrap_err();
91-
fadvise(&file, i64::MAX as u64, u64::MAX, Advice::Normal).unwrap_err();
92-
fadvise(&file, 0, u64::MAX, Advice::Normal).unwrap_err();
93-
fadvise(&file, u64::MAX, i64::MAX as u64 + 1, Advice::Normal).unwrap_err();
94-
fadvise(&file, i64::MAX as u64 + 1, u64::MAX, Advice::Normal).unwrap_err();
95-
fadvise(&file, i64::MAX as u64, i64::MAX as u64 + 1, Advice::Normal).unwrap_err();
96-
fadvise(&file, 0, i64::MAX as u64 + 1, Advice::Normal).unwrap_err();
103+
fadvise(&file, u64::MAX, NonZeroU64::new(u64::MAX), Advice::Normal).unwrap_err();
104+
fadvise(
105+
&file,
106+
i64::MAX as u64,
107+
NonZeroU64::new(u64::MAX),
108+
Advice::Normal,
109+
)
110+
.unwrap_err();
111+
fadvise(&file, 0, NonZeroU64::new(u64::MAX), Advice::Normal).unwrap_err();
112+
fadvise(
113+
&file,
114+
u64::MAX,
115+
NonZeroU64::new(i64::MAX as u64 + 1),
116+
Advice::Normal,
117+
)
118+
.unwrap_err();
119+
fadvise(
120+
&file,
121+
i64::MAX as u64 + 1,
122+
NonZeroU64::new(u64::MAX),
123+
Advice::Normal,
124+
)
125+
.unwrap_err();
126+
fadvise(
127+
&file,
128+
i64::MAX as u64,
129+
NonZeroU64::new(i64::MAX as u64 + 1),
130+
Advice::Normal,
131+
)
132+
.unwrap_err();
133+
fadvise(
134+
&file,
135+
0,
136+
NonZeroU64::new(i64::MAX as u64 + 1),
137+
Advice::Normal,
138+
)
139+
.unwrap_err();
97140
}
98141

99142
#[test]

‎tests/fs/negative_timestamp.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#[cfg(not(target_os = "redox"))]
12
#[test]
23
fn negative_file_timetamp() {
34
use rustix::fs::{

0 commit comments

Comments
 (0)