Skip to content

Commit 03d129a

Browse files
StebalienStebalien
authored
feat: use a larger buffer when reading attributes (#80)
The code is somewhat nasty, but I'm trying to abstract over different platforms and multiple calling conventions... fixes #79
1 parent 1ec2f59 commit 03d129a

File tree

3 files changed

+95
-108
lines changed

3 files changed

+95
-108
lines changed

‎src/sys/bsd.rs‎

Lines changed: 37 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,19 @@
22
33
use libc::{c_int, c_void, size_t, EPERM};
44
use std::ffi::{CString, OsStr, OsString};
5-
use std::io;
6-
use std::mem;
5+
use std::mem::{self, MaybeUninit};
76
use std::os::unix::ffi::{OsStrExt, OsStringExt};
87
use std::os::unix::io::{AsRawFd, BorrowedFd};
98
use std::path::Path;
109
use std::ptr;
10+
use std::{io, slice};
1111

1212
use libc::{
1313
extattr_delete_fd, extattr_delete_file, extattr_delete_link, extattr_get_fd, extattr_get_file,
1414
extattr_get_link, extattr_list_fd, extattr_list_file, extattr_list_link, extattr_set_fd,
1515
extattr_set_file, extattr_set_link, EXTATTR_NAMESPACE_SYSTEM, EXTATTR_NAMESPACE_USER,
1616
};
1717

18-
use crate::util::allocate_loop;
19-
2018
pub const ENOATTR: i32 = libc::ENOATTR;
2119
pub const ERANGE: i32 = libc::ERANGE;
2220

@@ -36,14 +34,38 @@ fn path_to_c(path: &Path) -> io::Result<CString> {
3634
}
3735

3836
#[inline]
39-
fn slice_parts(buf: &mut [u8]) -> (*mut c_void, size_t) {
37+
fn cvt(res: libc::ssize_t) -> io::Result<usize> {
38+
if res < 0 {
39+
Err(io::Error::last_os_error())
40+
} else {
41+
Ok(res as usize)
42+
}
43+
}
44+
45+
#[inline]
46+
fn slice_parts(buf: &mut [MaybeUninit<u8>]) -> (*mut c_void, size_t) {
4047
if buf.is_empty() {
4148
(ptr::null_mut(), 0)
4249
} else {
4350
(buf.as_mut_ptr().cast(), buf.len() as size_t)
4451
}
4552
}
4653

54+
fn allocate_loop<F: Fn(*mut c_void, size_t) -> libc::ssize_t>(f: F) -> io::Result<Vec<u8>> {
55+
crate::util::allocate_loop(
56+
|buf| unsafe {
57+
let (ptr, len) = slice_parts(buf);
58+
let new_len = cvt(f(ptr, len))?;
59+
assert!(
60+
new_len <= len,
61+
"OS returned length {new_len} greater than buffer size {len}"
62+
);
63+
Ok(slice::from_raw_parts_mut(ptr.cast(), new_len))
64+
},
65+
|| cvt(f(ptr::null_mut(), 0)),
66+
)
67+
}
68+
4769
/// An iterator over a set of extended attributes names.
4870
#[derive(Default)]
4971
pub struct XAttrs {
@@ -165,22 +187,9 @@ fn prefix_namespace(attr: &OsStr, ns: c_int) -> OsString {
165187
OsString::from_vec(v)
166188
}
167189

168-
fn cvt(res: libc::ssize_t) -> io::Result<usize> {
169-
if res < 0 {
170-
Err(io::Error::last_os_error())
171-
} else {
172-
Ok(res as usize)
173-
}
174-
}
175-
176190
pub fn get_fd(fd: BorrowedFd<'_>, name: &OsStr) -> io::Result<Vec<u8>> {
177191
let (ns, name) = name_to_ns(name)?;
178-
unsafe {
179-
allocate_loop(|buf| {
180-
let (ptr, len) = slice_parts(buf);
181-
cvt(extattr_get_fd(fd.as_raw_fd(), ns, name.as_ptr(), ptr, len))
182-
})
183-
}
192+
unsafe { allocate_loop(|ptr, len| extattr_get_fd(fd.as_raw_fd(), ns, name.as_ptr(), ptr, len)) }
184193
}
185194

186195
pub fn set_fd(fd: BorrowedFd<'_>, name: &OsStr, value: &[u8]) -> io::Result<()> {
@@ -213,14 +222,8 @@ pub fn remove_fd(fd: BorrowedFd<'_>, name: &OsStr) -> io::Result<()> {
213222

214223
pub fn list_fd(fd: BorrowedFd<'_>) -> io::Result<XAttrs> {
215224
let sysvec = unsafe {
216-
let res = allocate_loop(|buf| {
217-
let (ptr, len) = slice_parts(buf);
218-
cvt(extattr_list_fd(
219-
fd.as_raw_fd(),
220-
EXTATTR_NAMESPACE_SYSTEM,
221-
ptr,
222-
len,
223-
))
225+
let res = allocate_loop(|ptr, len| {
226+
extattr_list_fd(fd.as_raw_fd(), EXTATTR_NAMESPACE_SYSTEM, ptr, len)
224227
});
225228
// On FreeBSD, system attributes require root privileges to view. However,
226229
// to mimic the behavior of listxattr in linux and osx, we need to query
@@ -238,14 +241,8 @@ pub fn list_fd(fd: BorrowedFd<'_>) -> io::Result<XAttrs> {
238241
};
239242

240243
let uservec = unsafe {
241-
let res = allocate_loop(|buf| {
242-
let (ptr, len) = slice_parts(buf);
243-
cvt(extattr_list_fd(
244-
fd.as_raw_fd(),
245-
EXTATTR_NAMESPACE_USER,
246-
ptr,
247-
len,
248-
))
244+
let res = allocate_loop(|ptr, len| {
245+
extattr_list_fd(fd.as_raw_fd(), EXTATTR_NAMESPACE_USER, ptr, len)
249246
});
250247
match res {
251248
Ok(v) => v,
@@ -269,10 +266,7 @@ pub fn get_path(path: &Path, name: &OsStr, deref: bool) -> io::Result<Vec<u8>> {
269266
extattr_get_link
270267
};
271268
unsafe {
272-
allocate_loop(|buf| {
273-
let (ptr, len) = slice_parts(buf);
274-
cvt(extattr_get_func(path.as_ptr(), ns, name.as_ptr(), ptr, len))
275-
})
269+
allocate_loop(|ptr, len| extattr_get_func(path.as_ptr(), ns, name.as_ptr(), ptr, len))
276270
}
277271
}
278272

@@ -324,14 +318,8 @@ pub fn list_path(path: &Path, deref: bool) -> io::Result<XAttrs> {
324318
extattr_list_link
325319
};
326320
let sysvec = unsafe {
327-
let res = allocate_loop(|buf| {
328-
let (ptr, len) = slice_parts(buf);
329-
cvt(extattr_list_func(
330-
path.as_ptr(),
331-
EXTATTR_NAMESPACE_SYSTEM,
332-
ptr,
333-
len,
334-
))
321+
let res = allocate_loop(|ptr, len| {
322+
extattr_list_func(path.as_ptr(), EXTATTR_NAMESPACE_SYSTEM, ptr, len)
335323
});
336324
// On FreeBSD, system attributes require root privileges to view. However,
337325
// to mimic the behavior of listxattr in linux and osx, we need to query
@@ -349,14 +337,8 @@ pub fn list_path(path: &Path, deref: bool) -> io::Result<XAttrs> {
349337
};
350338

351339
let uservec = unsafe {
352-
let res = allocate_loop(|buf| {
353-
let (ptr, len) = slice_parts(buf);
354-
cvt(extattr_list_func(
355-
path.as_ptr(),
356-
EXTATTR_NAMESPACE_USER,
357-
ptr,
358-
len,
359-
))
340+
let res = allocate_loop(|ptr, len| {
341+
extattr_list_func(path.as_ptr(), EXTATTR_NAMESPACE_USER, ptr, len)
360342
});
361343
match res {
362344
Ok(v) => v,

‎src/sys/linux_macos.rs‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ use std::path::Path;
77

88
use rustix::fs as rfs;
99

10-
use crate::util::allocate_loop;
11-
1210
#[cfg(not(target_os = "macos"))]
1311
pub const ENOATTR: i32 = rustix::io::Errno::NODATA.raw_os_error();
1412

@@ -66,8 +64,23 @@ impl Iterator for XAttrs {
6664
}
6765
}
6866

67+
/// A macro to abstract away some of the boilerplate when calling `allocate_loop` with rustix
68+
/// functions. Unfortunately, we can't write this as a helper function because I need to call
69+
/// generic rustix function with two different types.
70+
macro_rules! allocate_loop {
71+
(|$buf:ident| $($e:tt)*) => {
72+
crate::util::allocate_loop(
73+
|$buf| Ok($($e)*?.0),
74+
|| {
75+
let $buf: &mut [u8] = &mut [];
76+
Ok($($e)*?)
77+
},
78+
)
79+
};
80+
}
81+
6982
pub fn get_fd(fd: BorrowedFd<'_>, name: &OsStr) -> io::Result<Vec<u8>> {
70-
allocate_loop(|buf| rfs::fgetxattr(fd, name, buf))
83+
allocate_loop!(|buf| rfs::fgetxattr(fd, name, buf))
7184
}
7285

7386
pub fn set_fd(fd: BorrowedFd<'_>, name: &OsStr, value: &[u8]) -> io::Result<()> {
@@ -81,19 +94,19 @@ pub fn remove_fd(fd: BorrowedFd<'_>, name: &OsStr) -> io::Result<()> {
8194
}
8295

8396
pub fn list_fd(fd: BorrowedFd<'_>) -> io::Result<XAttrs> {
84-
let vec = allocate_loop(|buf| rfs::flistxattr(fd, buf))?;
97+
let vec = allocate_loop!(|buf| rfs::flistxattr(fd, buf))?;
8598
Ok(XAttrs {
8699
data: vec.into_boxed_slice(),
87100
offset: 0,
88101
})
89102
}
90103

91104
pub fn get_path(path: &Path, name: &OsStr, deref: bool) -> io::Result<Vec<u8>> {
92-
allocate_loop(|buf| {
93-
let getxattr_func = if deref { rfs::getxattr } else { rfs::lgetxattr };
94-
let size = getxattr_func(path, name, buf)?;
95-
io::Result::Ok(size)
96-
})
105+
if deref {
106+
allocate_loop!(|buf| rfs::getxattr(path, name, buf))
107+
} else {
108+
allocate_loop!(|buf| rfs::lgetxattr(path, name, buf))
109+
}
97110
}
98111

99112
pub fn set_path(path: &Path, name: &OsStr, value: &[u8], deref: bool) -> io::Result<()> {
@@ -103,23 +116,20 @@ pub fn set_path(path: &Path, name: &OsStr, value: &[u8], deref: bool) -> io::Res
103116
}
104117

105118
pub fn remove_path(path: &Path, name: &OsStr, deref: bool) -> io::Result<()> {
106-
let removexattr_func = if deref {
107-
rfs::removexattr
119+
if deref {
120+
rfs::removexattr(path, name)
108121
} else {
109-
rfs::lremovexattr
110-
};
111-
removexattr_func(path, name)?;
122+
rfs::lremovexattr(path, name)
123+
}?;
112124
Ok(())
113125
}
114126

115127
pub fn list_path(path: &Path, deref: bool) -> io::Result<XAttrs> {
116-
let vec = allocate_loop(|buf| {
117-
if deref {
118-
rfs::listxattr(path, buf)
119-
} else {
120-
rfs::llistxattr(path, buf)
121-
}
122-
})?;
128+
let vec = if deref {
129+
allocate_loop!(|buf| rfs::listxattr(path, buf))
130+
} else {
131+
allocate_loop!(|buf| rfs::llistxattr(path, buf))
132+
}?;
123133
Ok(XAttrs {
124134
data: vec.into_boxed_slice(),
125135
offset: 0,

‎src/util.rs‎

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::io;
2+
use std::mem::MaybeUninit;
23

34
pub fn extract_noattr(result: io::Result<Vec<u8>>) -> io::Result<Option<Vec<u8>>> {
45
result.map(Some).or_else(|e| {
@@ -10,48 +11,42 @@ pub fn extract_noattr(result: io::Result<Vec<u8>>) -> io::Result<Option<Vec<u8>>
1011
})
1112
}
1213

14+
/// Calls `get_value` to with a buffer and `get_size` to estimate the size of the buffer if/when
15+
/// `get_value` returns ERANGE.
1316
#[allow(dead_code)]
14-
pub fn allocate_loop<E, F: FnMut(&mut [u8]) -> Result<usize, E>>(mut f: F) -> io::Result<Vec<u8>>
17+
pub fn allocate_loop<F, S>(mut get_value: F, mut get_size: S) -> io::Result<Vec<u8>>
1518
where
16-
io::Error: From<E>,
19+
F: for<'a> FnMut(&'a mut [MaybeUninit<u8>]) -> io::Result<&'a mut [u8]>,
20+
S: FnMut() -> io::Result<usize>,
1721
{
18-
// Try assuming the variable is less than 256. We do this on the stack and copy to avoid double
19-
// allocation.
20-
const INITIAL_BUFFER_SIZE: usize = 256;
21-
{
22-
let mut buf = [0u8; INITIAL_BUFFER_SIZE];
23-
match f(&mut buf[..]) {
24-
Ok(len) => return Ok(buf[..len].to_vec()),
25-
Err(e) => {
26-
let err: io::Error = e.into();
27-
if err.raw_os_error() != Some(crate::sys::ERANGE) {
28-
return Err(err);
29-
}
30-
}
31-
}
22+
// Start by assuming the return value is <= 4KiB. If it is, we can do this in one syscall.
23+
const INITIAL_BUFFER_SIZE: usize = 4096;
24+
match get_value(&mut [MaybeUninit::<u8>::uninit(); INITIAL_BUFFER_SIZE]) {
25+
Ok(val) => return Ok(val.to_vec()),
26+
Err(e) if e.raw_os_error() != Some(crate::sys::ERANGE) => return Err(e),
27+
_ => {}
3228
}
3329

34-
// If that fails, enter the allocation loop.
30+
// If that fails, we ask for the size and try again with a buffer of the correct size.
3531
let mut vec: Vec<u8> = Vec::new();
3632
loop {
37-
let ret = f(&mut [])?;
38-
vec.resize(ret, 0);
39-
40-
match f(&mut vec) {
41-
Ok(size) => {
42-
vec.truncate(size);
33+
vec.reserve_exact(get_size()?);
34+
match get_value(vec.spare_capacity_mut()) {
35+
Ok(initialized) => {
36+
unsafe {
37+
let len = initialized.len();
38+
assert_eq!(
39+
initialized.as_ptr(),
40+
vec.as_ptr(),
41+
"expected the same buffer"
42+
);
43+
vec.set_len(len);
44+
}
4345
vec.shrink_to_fit();
4446
return Ok(vec);
4547
}
46-
47-
Err(e) => {
48-
let err: io::Error = e.into();
49-
if err.raw_os_error() == Some(crate::sys::ERANGE) {
50-
continue;
51-
} else {
52-
return Err(err);
53-
}
54-
}
48+
Err(e) if e.raw_os_error() != Some(crate::sys::ERANGE) => return Err(e),
49+
_ => {} // try again
5550
}
5651
}
5752
}

0 commit comments

Comments
 (0)