Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
- Add `--exact` option to match the entire filename exactly (literal, non-substring).

## Bugfixes
- Reject `--threads` values above 64 instead of panicking during resource
allocation, see #2078 (@Sushanth012).
- Sanitize control characters and bidirectional override characters in filenames
when output goes to a terminal, to prevent terminal escape-sequence injection.
Also reject a placeholder as the executable for `--exec-batch`, while still
Expand Down
30 changes: 28 additions & 2 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use crate::filesystem;
use crate::filter::OwnerFilter;
use crate::filter::SizeFilter;

const MAX_NUM_THREADS: usize = 64;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 64?

The number of threads can be significantly higher than that. Especially on hosts with a large number of cpu cores.


#[derive(Parser)]
#[command(
name = "fd",
Expand Down Expand Up @@ -553,7 +555,7 @@ pub struct Opts {

/// Set number of threads to use for searching & executing (default: number
/// of available CPU cores)
#[arg(long, short = 'j', value_name = "num", hide_short_help = true, value_parser = str::parse::<NonZeroUsize>)]
#[arg(long, short = 'j', value_name = "num", hide_short_help = true, value_parser = parse_num_threads)]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use clap's built-in support for ranges?

pub threads: Option<NonZeroUsize>,

/// Milliseconds to buffer before streaming search results to console
Expand Down Expand Up @@ -792,13 +794,25 @@ fn default_num_threads() -> NonZeroUsize {
let fallback = NonZeroUsize::MIN;
// To limit startup overhead on massively parallel machines, don't use more
// than 64 threads.
let limit = NonZeroUsize::new(64).unwrap();
let limit = NonZeroUsize::new(MAX_NUM_THREADS).unwrap();

std::thread::available_parallelism()
.unwrap_or(fallback)
.min(limit)
}

fn parse_num_threads(arg: &str) -> Result<NonZeroUsize, String> {
let threads = arg
.parse::<NonZeroUsize>()
.map_err(|error| error.to_string())?;
if threads.get() > MAX_NUM_THREADS {
return Err(format!(
"the number of threads cannot exceed {MAX_NUM_THREADS}"
));
}
Ok(threads)
}

#[derive(Copy, Clone, PartialEq, Eq, ValueEnum)]
pub enum FileType {
#[value(alias = "f")]
Expand Down Expand Up @@ -969,3 +983,15 @@ fn ensure_current_directory_exists(current_directory: &Path) -> anyhow::Result<(
))
}
}

#[cfg(test)]
mod tests {
use super::parse_num_threads;

#[test]
fn number_of_threads_is_bounded() {
assert_eq!(parse_num_threads("64").unwrap().get(), 64);
assert!(parse_num_threads("65").is_err());
assert!(parse_num_threads("9223372036854775807").is_err());
}
}
2 changes: 2 additions & 0 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2659,6 +2659,8 @@ fn test_number_parsing_errors() {
te.assert_failure(&["--threads=a"]);
te.assert_failure(&["-j", ""]);
te.assert_failure(&["--threads=0"]);
te.assert_failure(&["--threads=65"]);
te.assert_failure(&["--threads=9223372036854775807"]);

te.assert_failure(&["--min-depth=a"]);
te.assert_failure(&["--mindepth=a"]);
Expand Down
Loading