[KEIAHNY]

PR Type

Enhancement, Documentation

---

Description

• Add init command generating default config file

• Support CPU ranges in max_cpu_usage_vec parsing

  • Validate CPUs against available system cores
  • Deduplicate expanded CPU lists

• Bump version to 1.1.0-dev with docs updates

• Specify required Suricata branch and commit

---

Diagram Walkthrough

flowchart LR
  init["Init command"] -- "generates" --> config["Default suriconf.yaml"]
  cli["CLI max_cpu_usage_vec"] -- "strings with ranges" --> parse["parse_cpu_list"]
  yaml["YAML max_cpu_usage_vec"] -- "numbers or ranges" --> parse
  parse -- "expands & validates" --> cpus["Deduplicated CPU vector"]

Loading

File Walkthrough

	Relevant files
Enhancement	5 files yaml.rs Add CPU range expansion with validation and deduplication +76/-13  argument.rs Add Init command and change CPU vec to strings                      +14/-2    main.rs Handle Init command before main workflow                                  +10/-1    config_gen.rs New module generating default configuration file                  +16/-0    lib.rs Register config_gen module and embed default config            +4/-1
Formatting	1 files suricata.rs Simplify delete_pid_file return type via io import              +2/-1
Documentation	1 files README.md Update version, Suricata spec, and init usage docs              +14/-8
Configuration changes	1 files suriconf.yaml Bump version and update default configuration values          +4/-4
Dependencies	1 files Cargo.toml Bump version to 1.1.0-dev, add num_cpus dependency              +3/-2
Additional files	4 files ISSUES.md +0/-3      suricata.yaml +0/-2347 suricata.yaml.in +0/-2339 suriconf.yaml.in +0/-6

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The init command handles failures with panic!("{e}"). When a user runs suriconf init without --force and the file already exists (a very common scenario), they get a full panic message and backtrace with exit code 101 instead of a clean error on stderr and a conventional non-zero exit code. The error string from config_gen::init (including the helpful use --force hint) is better reported via eprintln! and process::exit(1). if let Err(e) = init(output, *force) { panic!("{e}"); } Panic on Config find_max_cpu_usage_vec errors are converted to panic! at the call site. Combined with the new CPU validation against num_cpus::get(), any previously working configuration that lists a CPU index >= the visible CPU count (e.g., when running under taskset, in a container, or on a smaller machine than where the config was authored) now aborts the program with a panic instead of being handled gracefully. Consider returning a Result up to main for a clean error message. self.max_cpu_usage_vec = match result { Ok(cpus) => cpus, Err(msg) => panic!("Unable to parse max cpu usage vector: {msg}"), };

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Replace panic with graceful error exit for init command Using panic! for a user-facing CLI error (e.g., the file already exists without --force) prints an ugly panic message and backtrace, and exits with code 101 instead of a standard error code. Print the error to stderr and exit with status 1 instead. src/main.rs [26-31] if let Some(Commands::Init { output, force }) = &args.cmd { if let Err(e) = init(output, *force) { - panic!("{e}"); + eprintln!("Error: {e}"); + process::exit(1); } process::exit(0); } Suggestion importance[1-10]: 6 __ Why: Using panic! for an expected user-facing CLI error (e.g., file exists without --force) produces an ugly backtrace and exit code 101; printing to stderr and exiting with code 1 is the correct CLI behavior. Correct and useful, but a minor UX improvement rather than a critical bug.	Low
	Deduplicate CPU list parsing logic via shared helper The deduplication and expansion loop here duplicates the logic in parse_cpu_list. Extract a shared helper that inserts a single CPU specification (deduplicating via the seen set) and reuse it from both functions, so future fixes apply in one place only. src/yaml.rs [1006-1021] + fn insert_cpu(s: &str, seen: &mut HashSet<u64>, cpus: &mut Vec<u64>) -> Result<(), String> { + for cpu in expand_cpu_range(s)? { + if seen.insert(cpu) { + cpus.push(cpu); + } + } + Ok(()) + } + + fn parse_cpu_list(items: &[String]) -> Result<Vec<u64>, String> { + let mut cpus = Vec::new(); + let mut seen = HashSet::new(); + for item in items { + insert_cpu(item, &mut seen, &mut cpus)?; + } + Ok(cpus) + } + + // in find_max_cpu_usage_vec: for item in seq { let s = match item { Value::Number(n) => n.to_string(), Value::String(s) => s.clone(), - u => return Err(format!("Invalid CPU {:?}.", u)) - + u => return Err(format!("Invalid CPU {:?}.", u)), }; - - for cpu in expand_cpu_range(&s)? { - if seen.insert(cpu) { - cpus.push(cpu); - } - } - + insert_cpu(&s, &mut seen, &mut cpus)?; } Ok(cpus) Suggestion importance[1-10]: 4 __ Why: The deduplication/expansion loop is indeed duplicated between find_max_cpu_usage_vec and parse_cpu_list, and the shared insert_cpu helper would improve maintainability. This is a purely stylistic refactor with no functional or correctness impact.	Low
	Validate malformed CPU range inputs before parsing Inputs such as -2 or 0--3 are split by the first hyphen, producing an empty start segment and a confusing error like Invalid CPU .. Validate the input shape (e.g., reject empty parts or a leading hyphen) before splitting so error messages are meaningful. src/yaml.rs [1074-1075] - Some((start, end)) => { - let start: u64 = start.trim().parse().map_err(|_| format!("Invalid CPU {}.", start))?; + let s = s.trim(); + if s.is_empty() || s.starts_with('-') || s.contains("--") || s.ends_with('-') { + return Err(format!("Invalid CPU specification '{}'.", s)); + } + match s.split_once('-') { Suggestion importance[1-10]: 4 __ Why: Malformed inputs like -2 do split into an empty start segment producing the confusing message Invalid CPU ., so the observation is accurate. The fix only improves error message quality for invalid user input, a minor robustness enhancement.	Low
Security	Prevent race condition when refusing to overwrite file Checking output.exists() and then writing has a TOCTOU race: the file could be created between the check and fs::write, silently overwriting it. Use OpenOptions with create_new(true) to make the no-overwrite guarantee atomic, and map the AlreadyExists error to the friendly message. src/config_gen.rs [7-16] pub fn init(output: &PathBuf, force: bool) -> io::Result<()> { - if output.exists() && !force { - return Err(io::Error::new( - io::ErrorKind::AlreadyExists, - format!("refusing to overwrite {:?} (use --force)", output), - )); - } - fs::write(output, DEFAULT_CONFIG)?; + use std::fs::OpenOptions; + use std::io::Write; + + let mut file = if force { + OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(output)? + } else { + OpenOptions::new() + .write(true) + .create_new(true) + .open(output) + .map_err(|e| { + if e.kind() == io::ErrorKind::AlreadyExists { + io::Error::new( + io::ErrorKind::AlreadyExists, + format!("refusing to overwrite {:?} (use --force)", output), + ) + } else { + e + } + })? + }; + + file.write_all(DEFAULT_CONFIG)?; Ok(()) } Suggestion importance[1-10]: 6 __ Why: The TOCTOU race between output.exists() and fs::write is real, and using OpenOptions::create_new with error mapping makes the no-overwrite guarantee atomic. The fix is accurate, though the practical impact in a single-user CLI tool is moderate.	Low

• Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

[KEIAHNY]

Closing - applying changes.