Skip to content

Commit dddf0a3

Browse files
oech3sylvestre
authored andcommitted
pipes.rs: avoid usage of bool
1 parent 4f9a39b commit dddf0a3

3 files changed

Lines changed: 26 additions & 38 deletions

File tree

src/uu/tail/src/tail.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ fn print_target_section<
596596
}
597597
} else {
598598
#[cfg(any(target_os = "linux", target_os = "android"))]
599-
if uucore::pipes::splice_unbounded_broker(file, &mut stdout)? {
599+
if uucore::pipes::splice_unbounded_broker(file, &mut stdout)?.is_err() {
600600
io::copy(file, &mut stdout)?;
601601
}
602602
#[cfg(not(any(target_os = "linux", target_os = "android")))]

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

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,7 @@
44
// file that was distributed with this source code.
55

66
//! Buffer-based copying implementation for Linux and Android.
7-
8-
use crate::error::UResult;
9-
10-
/// Buffer-based copying utilities for unix (excluding Linux).
11-
use std::{
12-
io::{Read, Write},
13-
os::fd::AsFd,
14-
};
7+
use std::os::fd::AsFd;
158

169
/// Copy data from `Read` implementor `source` into a `Write` implementor
1710
/// `dest`. This works by reading a chunk of data from `source` and writing the
@@ -24,26 +17,16 @@ use std::{
2417
/// # Arguments
2518
/// * `source` - `Read` implementor to copy data from.
2619
/// * `dest` - `Write` implementor to copy data to.
27-
pub fn copy_stream<R, S>(src: &mut R, dest: &mut S) -> UResult<()>
28-
where
29-
R: Read + AsFd,
30-
S: Write + AsFd,
31-
{
20+
pub fn copy_stream(
21+
src: &mut (impl std::io::Read + AsFd),
22+
dest: &mut impl AsFd,
23+
) -> crate::error::UResult<()> {
3224
// If we're on Linux or Android, try to use the splice() system call
3325
// for faster writing. If it works, we're done.
34-
// todo: bypass broker pipe this if input or output is pipe. We use this mostly for stream.
35-
if !crate::pipes::splice_unbounded_broker(src, dest)? {
36-
return Ok(());
26+
if crate::pipes::splice_unbounded_auto(src, dest)? {
27+
// If the splice() call failed, fall back on writing "without buffering", or order of output would be wrong
28+
// unrelated for cp /dev/stdin since cp does not have multiple input? <https://github.com/uutils/coreutils/issues/5186>
29+
std::io::copy(src, &mut crate::io::RawWriter(dest))?;
3730
}
38-
39-
// If the splice() call failed, fall back on slower writing.
40-
std::io::copy(src, dest)?;
41-
42-
// If the splice() call failed and there has been some data written to
43-
// stdout via while loop above AND there will be second splice() call
44-
// that will succeed, data pushed through splice will be output before
45-
// the data buffered in stdout.lock. Therefore additional explicit flush
46-
// is required here.
47-
dest.flush()?;
4831
Ok(())
4932
}

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

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -100,19 +100,24 @@ pub fn splice_unbounded(source: &impl AsFd, dest: &mut impl AsFd) -> rustix::io:
100100
Ok(())
101101
}
102102

103-
/// force-splice source to dest even both of them are not pipe
104-
/// return true if we need read/write fallback
103+
/// force-splice source to dest even both of them are not pipe via broker pipe
104+
/// returns Ok(Ok(())) if splice succeeds
105+
/// returns Ok(Err()) if splice failed, but you can fallback to read/write
106+
/// returns std::io::Result if splice from broker failed and read/write fallback from broker failed
105107
///
108+
/// Thus, ?.is_err() returns serious error at early stage and checks that you can fallback
106109
/// This should not be used if one of them are pipe to save resources
107110
#[inline]
108111
#[cfg(any(target_os = "linux", target_os = "android"))]
109-
pub fn splice_unbounded_broker(source: &impl AsFd, dest: &mut impl AsFd) -> std::io::Result<bool> {
112+
pub fn splice_unbounded_broker(
113+
source: &impl AsFd,
114+
dest: &mut impl AsFd,
115+
) -> std::io::Result<Result<(), ()>> {
110116
static PIPE_CACHE: OnceLock<Option<(PipeReader, PipeWriter)>> = OnceLock::new();
111-
let Some((pipe_rd, pipe_wr)) = PIPE_CACHE
112-
.get_or_init(|| pipe::<false>(MAX_ROOTLESS_PIPE_SIZE).ok())
113-
.as_ref()
117+
let Some((pipe_rd, pipe_wr)) =
118+
PIPE_CACHE.get_or_init(|| pipe::<false>(MAX_ROOTLESS_PIPE_SIZE).ok())
114119
else {
115-
return Ok(true);
120+
return Ok(Err(()));
116121
};
117122
// improve throughput
118123
// no need to increase pipe size of input fd since
@@ -122,7 +127,7 @@ pub fn splice_unbounded_broker(source: &impl AsFd, dest: &mut impl AsFd) -> std:
122127

123128
loop {
124129
match splice(&source, &pipe_wr, MAX_ROOTLESS_PIPE_SIZE) {
125-
Ok(0) => return Ok(false),
130+
Ok(0) => return Ok(Ok(())),
126131
Ok(n) => {
127132
if splice_exact(&pipe_rd, dest, n).is_err() {
128133
// If the first splice manages to copy to the intermediate
@@ -135,10 +140,10 @@ pub fn splice_unbounded_broker(source: &impl AsFd, dest: &mut impl AsFd) -> std:
135140
let mut drain = Vec::with_capacity(n);
136141
pipe_rd.take(n as u64).read_to_end(&mut drain)?;
137142
RawWriter(&dest).write_all(&drain)?;
138-
return Ok(true);
143+
return Ok(Err(()));
139144
}
140145
}
141-
Err(_) => return Ok(true),
146+
Err(_) => return Ok(Err(())),
142147
}
143148
}
144149
}
@@ -153,7 +158,7 @@ pub fn splice_unbounded_auto(source: &impl AsFd, dest: &mut impl AsFd) -> std::i
153158
// use splice to check that input or output is pipe which is efficient
154159
let fallback = match splice(&source, dest, MAX_ROOTLESS_PIPE_SIZE) {
155160
Ok(_) => splice_unbounded(source, dest).is_err(),
156-
_ => splice_unbounded_broker(source, dest)?,
161+
_ => splice_unbounded_broker(source, dest)?.is_err(),
157162
};
158163
Ok(fallback)
159164
}

0 commit comments

Comments
 (0)