Skip to content

Commit 364700e

Browse files
committed
refactor: use SpliceState enum instead of bool in splice functions
Improve code clarity by using explicit enum values instead of boolean semantics
1 parent c170a18 commit 364700e

5 files changed

Lines changed: 51 additions & 32 deletions

File tree

src/uu/cat/src/cat.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,8 +487,9 @@ fn print_fast<R: FdReadable>(handle: &mut InputHandle<R>) -> CatResult<()> {
487487
{
488488
// If we're on Linux or Android, try to use the splice() system call
489489
// for faster writing. If it works, we're done.
490-
if !splice::write_fast_using_splice(handle, &mut stdout)? {
491-
return Ok(());
490+
match splice::write_fast_using_splice(handle, &mut stdout)? {
491+
uucore::pipes::SpliceState::Ended => return Ok(()),
492+
uucore::pipes::SpliceState::Fallback => {}
492493
}
493494
}
494495
// If we're not on Linux or Android, or the splice() call failed,

src/uu/cat/src/splice.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,23 @@ use uucore::pipes::{MAX_ROOTLESS_PIPE_SIZE, might_fuse, splice};
1313
/// without copying between kernel and user spaces. This results in a large
1414
/// speedup.
1515
///
16-
/// The `bool` in the result value indicates if we need to fall back to normal
17-
/// copying or not. False means we don't have to.
16+
/// The `SpliceState` in the result value indicates if we need to fall back to
17+
/// normal copying or not. `SpliceState::Ended` means we don't have to.
1818
#[inline]
1919
pub(super) fn write_fast_using_splice<R: FdReadable, S: AsFd>(
2020
handle: &InputHandle<R>,
2121
write_fd: &mut S,
22-
) -> CatResult<bool> {
23-
let res = match splice(&handle.reader, &write_fd, MAX_ROOTLESS_PIPE_SIZE) {
22+
) -> CatResult<uucore::pipes::SpliceState> {
23+
let splice_state = match splice(&handle.reader, &write_fd, MAX_ROOTLESS_PIPE_SIZE) {
2424
Ok(_) => uucore::pipes::splice_unbounded(&handle.reader, write_fd)?,
2525
// both of in/output are not pipe
2626
_ => uucore::pipes::splice_unbounded_broker(&handle.reader, write_fd)?,
2727
};
28-
Ok(res || might_fuse(&handle.reader))
28+
let final_state = match splice_state {
29+
uucore::pipes::SpliceState::Ended if might_fuse(reader) => {
30+
uucore::pipes::SpliceState::Fallback
31+
}
32+
state => state,
33+
};
34+
Ok(final_state)
2935
}

src/uu/tail/src/tail.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,13 @@ fn print_target_section<
591591
} else {
592592
// zero-copy fast-path
593593
#[cfg(any(target_os = "linux", target_os = "android"))]
594-
if uucore::pipes::splice_unbounded_broker(file, &mut stdout)? {
595-
io::copy(file, &mut stdout)?;
594+
{
595+
match uucore::pipes::splice_unbounded_broker(file, &mut stdout)? {
596+
uucore::pipes::SpliceState::Ended => return Ok(()),
597+
uucore::pipes::SpliceState::Fallback => {
598+
io::copy(file, &mut stdout)?;
599+
}
600+
}
596601
}
597602
#[cfg(not(any(target_os = "linux", target_os = "android")))]
598603
io::copy(file, &mut stdout)?;

src/uucore/src/lib/features/buf_copy/linux.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,18 @@ where
4242
// If we're on Linux or Android, try to use the splice() system call
4343
// for faster writing. If it works, we're done.
4444
// todo: bypass broker pipe this if input or output is pipe. We use this mostly for stream.
45-
if !crate::pipes::splice_unbounded_broker(src, dest)? {
46-
return Ok(());
45+
match crate::pipes::splice_unbounded_broker(src, dest)? {
46+
crate::pipes::SpliceState::Ended => Ok(()),
47+
crate::pipes::SpliceState::Fallback => {
48+
std::io::copy(src, dest)?;
49+
50+
// If the splice() call failed and there has been some data written to
51+
// stdout via while loop above AND there will be second splice() call
52+
// that will succeed, data pushed through splice will be output before
53+
// the data buffered in stdout.lock. Therefore additional explicit flush
54+
// is required here.
55+
dest.flush()?;
56+
Ok(())
57+
}
4758
}
48-
49-
// If the splice() call failed, fall back on slower writing.
50-
std::io::copy(src, dest)?;
51-
52-
// If the splice() call failed and there has been some data written to
53-
// stdout via while loop above AND there will be second splice() call
54-
// that will succeed, data pushed through splice will be output before
55-
// the data buffered in stdout.lock. Therefore additional explicit flush
56-
// is required here.
57-
dest.flush()?;
58-
Ok(())
5959
}

src/uucore/src/lib/features/pipes.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ pub fn pipe<const SIZE_REQUIRED: bool>(s: usize) -> std::io::Result<(File, File)
4040
Ok((File::from(read), File::from(write)))
4141
}
4242

43+
#[cfg(any(target_os = "linux", target_os = "android"))]
44+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
45+
pub enum SpliceState {
46+
Ended,
47+
Fallback,
48+
}
49+
4350
/// Less noisy wrapper around [`rustix::pipe::splice`].
4451
///
4552
/// Up to `len` bytes are moved from `source` to `target`. Returns the number
@@ -81,11 +88,11 @@ pub fn might_fuse(source: &impl AsFd) -> bool {
8188
}
8289

8390
/// splice all of source to dest
84-
/// return true if we need read/write fallback
91+
/// return SpliceState indicating if we need read/write fallback
8592
/// fails if one of in/output should be pipe
8693
#[inline]
8794
#[cfg(any(target_os = "linux", target_os = "android"))]
88-
pub fn splice_unbounded<R, S>(source: &R, dest: &mut S) -> std::io::Result<bool>
95+
pub fn splice_unbounded<R, S>(source: &R, dest: &mut S) -> std::io::Result<SpliceState>
8996
where
9097
R: Read + AsFd,
9198
S: AsFd,
@@ -99,19 +106,19 @@ where
99106
loop {
100107
match splice(&source, &dest, MAX_ROOTLESS_PIPE_SIZE) {
101108
Ok(1..) => {}
102-
Ok(0) => return Ok(false),
103-
Err(_) => return Ok(true),
109+
Ok(0) => return Ok(SpliceState::Ended),
110+
Err(_) => return Ok(SpliceState::Fallback),
104111
}
105112
}
106113
}
107114

108115
/// force-splice source to dest even both of them are not pipe
109-
/// return true if we need read/write fallback
116+
/// return SpliceState indicating if we need read/write fallback
110117
///
111118
/// This should not be used if one of them are pipe to save resources
112119
#[inline]
113120
#[cfg(any(target_os = "linux", target_os = "android"))]
114-
pub fn splice_unbounded_broker<R, S>(source: &R, dest: &mut S) -> std::io::Result<bool>
121+
pub fn splice_unbounded_broker<R, S>(source: &R, dest: &mut S) -> std::io::Result<SpliceState>
115122
where
116123
R: Read + AsFd,
117124
S: AsFd,
@@ -121,7 +128,7 @@ where
121128
.get_or_init(|| pipe::<false>(MAX_ROOTLESS_PIPE_SIZE).ok())
122129
.as_ref()
123130
else {
124-
return Ok(true);
131+
return Ok(SpliceState::Fallback);
125132
};
126133
// improve throughput
127134
// no need to increase pipe size of input fd since
@@ -131,7 +138,7 @@ where
131138

132139
loop {
133140
match splice(&source, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) {
134-
Ok(0) => return Ok(false),
141+
Ok(0) => return Ok(SpliceState::Ended),
135142
Ok(n) => {
136143
if splice_exact(&pipe_rd, dest, n).is_err() {
137144
// If the first splice manages to copy to the intermediate
@@ -143,10 +150,10 @@ where
143150
let mut drain = Vec::with_capacity(n);
144151
pipe_rd.take(n as u64).read_to_end(&mut drain)?;
145152
crate::io::RawWriter(&dest).write_all(&drain)?;
146-
return Ok(true);
153+
return Ok(SpliceState::Fallback);
147154
}
148155
}
149-
Err(_) => return Ok(true),
156+
Err(_) => return Ok(SpliceState::Fallback),
150157
}
151158
}
152159
}

0 commit comments

Comments
 (0)