Skip to content

Commit

Permalink
Fix fcntl_setpipe_size return value
Browse files Browse the repository at this point in the history
`fcntl` man page says that:
> F_SETPIPE_SZ ... The actual capacity (in bytes) that is set is
> returned as the function result.

But the current `fcntl_setpipe_size` function assumes that the return
value is 0 (success) or negative (failure). This makes the function
panic due to `debug_assert!` in debug mode, and it returns `Err` on
success in release mode.

This commit fixes the return value type and its checking, and adds a
test for it.
  • Loading branch information
jjyyxx committed Sep 15, 2024
1 parent f6f19e0 commit 64ba049
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/backend/libc/pipe/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ pub(crate) fn fcntl_getpipe_sz(fd: BorrowedFd<'_>) -> io::Result<usize> {

#[cfg(linux_kernel)]
#[inline]
pub(crate) fn fcntl_setpipe_sz(fd: BorrowedFd<'_>, size: usize) -> io::Result<()> {
pub(crate) fn fcntl_setpipe_sz(fd: BorrowedFd<'_>, size: usize) -> io::Result<usize> {
let size: c::c_int = size.try_into().map_err(|_| io::Errno::PERM)?;

unsafe { ret(c::fcntl(borrowed_fd(fd), c::F_SETPIPE_SZ, size)) }
unsafe { ret_c_int(c::fcntl(borrowed_fd(fd), c::F_SETPIPE_SZ, size)).map(|size| size as usize) }
}
6 changes: 3 additions & 3 deletions src/backend/linux_raw/pipe/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ pub(crate) fn fcntl_getpipe_sz(fd: BorrowedFd<'_>) -> io::Result<usize> {
}

#[inline]
pub(crate) fn fcntl_setpipe_sz(fd: BorrowedFd<'_>, size: usize) -> io::Result<()> {
pub(crate) fn fcntl_setpipe_sz(fd: BorrowedFd<'_>, size: usize) -> io::Result<usize> {
let size: c::c_int = size.try_into().map_err(|_| io::Errno::PERM)?;

#[cfg(target_pointer_width = "32")]
unsafe {
ret(syscall_readonly!(
ret_usize(syscall_readonly!(
__NR_fcntl64,
fd,
c_uint(F_SETPIPE_SZ),
Expand All @@ -125,7 +125,7 @@ pub(crate) fn fcntl_setpipe_sz(fd: BorrowedFd<'_>, size: usize) -> io::Result<()
}
#[cfg(target_pointer_width = "64")]
unsafe {
ret(syscall_readonly!(
ret_usize(syscall_readonly!(
__NR_fcntl,
fd,
c_uint(F_SETPIPE_SZ),
Expand Down
2 changes: 1 addition & 1 deletion src/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,6 @@ pub fn fcntl_getpipe_size<Fd: AsFd>(fd: Fd) -> io::Result<usize> {
/// [Linux]: https://man7.org/linux/man-pages/man2/fcntl.2.html
#[cfg(linux_kernel)]
#[inline]
pub fn fcntl_setpipe_size<Fd: AsFd>(fd: Fd, size: usize) -> io::Result<()> {
pub fn fcntl_setpipe_size<Fd: AsFd>(fd: Fd, size: usize) -> io::Result<usize> {
backend::pipe::syscalls::fcntl_setpipe_sz(fd.as_fd(), size)
}
31 changes: 31 additions & 0 deletions tests/pipe/fcntl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#[cfg(linux_kernel)]
#[test]
fn test_fcntl_getpipe_size() {
use rustix::pipe::fcntl_getpipe_size;

let (reader, writer) = rustix::pipe::pipe().unwrap();

let reader_size = fcntl_getpipe_size(&reader).unwrap();
let writer_size = fcntl_getpipe_size(&writer).unwrap();
assert_eq!(reader_size, writer_size);
}

#[cfg(linux_kernel)]
#[test]
fn test_fcntl_setpipe_size() {
use rustix::pipe::{fcntl_getpipe_size, fcntl_setpipe_size};

let (reader, writer) = rustix::pipe::pipe().unwrap();

let new_size = 4096 * 2;
let reader_size = fcntl_setpipe_size(&reader, new_size).unwrap();
let writer_size = fcntl_getpipe_size(&writer).unwrap();
assert_eq!(reader_size, new_size);
assert_eq!(reader_size, writer_size);

let new_size = 4096 * 16;
let reader_size = fcntl_setpipe_size(&reader, new_size).unwrap();
let writer_size = fcntl_getpipe_size(&writer).unwrap();
assert_eq!(reader_size, new_size);
assert_eq!(reader_size, writer_size);
}
1 change: 1 addition & 0 deletions tests/pipe/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
mod basic;
mod splice;
mod tee;
mod fcntl;

0 comments on commit 64ba049

Please sign in to comment.