Skip to content

Commit 2ce97b5

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 2ce97b5

5 files changed

Lines changed: 51 additions & 33 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(&handle.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 & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ 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+
pub enum SpliceState {
45+
Ended,
46+
Fallback,
47+
}
48+
4349
/// Less noisy wrapper around [`rustix::pipe::splice`].
4450
///
4551
/// Up to `len` bytes are moved from `source` to `target`. Returns the number
@@ -81,11 +87,11 @@ pub fn might_fuse(source: &impl AsFd) -> bool {
8187
}
8288

8389
/// splice all of source to dest
84-
/// return true if we need read/write fallback
85-
/// fails if one of in/output should be pipe
90+
/// return `SpliceState` indicating if we need read/write fallback
91+
/// fallback if one of in/output should be pipe
8692
#[inline]
8793
#[cfg(any(target_os = "linux", target_os = "android"))]
88-
pub fn splice_unbounded<R, S>(source: &R, dest: &mut S) -> std::io::Result<bool>
94+
pub fn splice_unbounded<R, S>(source: &R, dest: &mut S) -> std::io::Result<SpliceState>
8995
where
9096
R: Read + AsFd,
9197
S: AsFd,
@@ -99,19 +105,19 @@ where
99105
loop {
100106
match splice(&source, &dest, MAX_ROOTLESS_PIPE_SIZE) {
101107
Ok(1..) => {}
102-
Ok(0) => return Ok(false),
103-
Err(_) => return Ok(true),
108+
Ok(0) => return Ok(SpliceState::Ended),
109+
Err(_) => return Ok(SpliceState::Fallback),
104110
}
105111
}
106112
}
107113

108114
/// force-splice source to dest even both of them are not pipe
109-
/// return true if we need read/write fallback
115+
/// return `SpliceState` indicating if we need read/write fallback
110116
///
111117
/// This should not be used if one of them are pipe to save resources
112118
#[inline]
113119
#[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>
120+
pub fn splice_unbounded_broker<R, S>(source: &R, dest: &mut S) -> std::io::Result<SpliceState>
115121
where
116122
R: Read + AsFd,
117123
S: AsFd,
@@ -121,7 +127,7 @@ where
121127
.get_or_init(|| pipe::<false>(MAX_ROOTLESS_PIPE_SIZE).ok())
122128
.as_ref()
123129
else {
124-
return Ok(true);
130+
return Ok(SpliceState::Fallback);
125131
};
126132
// improve throughput
127133
// no need to increase pipe size of input fd since
@@ -131,7 +137,7 @@ where
131137

132138
loop {
133139
match splice(&source, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) {
134-
Ok(0) => return Ok(false),
140+
Ok(0) => return Ok(SpliceState::Ended),
135141
Ok(n) => {
136142
if splice_exact(&pipe_rd, dest, n).is_err() {
137143
// If the first splice manages to copy to the intermediate
@@ -143,10 +149,10 @@ where
143149
let mut drain = Vec::with_capacity(n);
144150
pipe_rd.take(n as u64).read_to_end(&mut drain)?;
145151
crate::io::RawWriter(&dest).write_all(&drain)?;
146-
return Ok(true);
152+
return Ok(SpliceState::Fallback);
147153
}
148154
}
149-
Err(_) => return Ok(true),
155+
Err(_) => return Ok(SpliceState::Fallback),
150156
}
151157
}
152158
}

0 commit comments

Comments
 (0)