Refactor port scanning and improve batch size handling - #905
Merged
Merged
Conversation
Refactor port scanning logic to improve performance and fix bugs related to batch size and ulimit handling. Update warnings and logging for better clarity.
bee-san
pushed a commit
that referenced
this pull request
Oct 1, 2026
…ulimit (#921) On native Windows, use the configured `-b/--batch-size` instead of a hard-coded 3000, and report the effective batch size (not the raw CLI value) in the "no open ports" hint on all platforms. `--ulimit` is hidden from Windows help and rejected on Windows (exit code 2), including when it comes from the config file. Unix batch-size/ulimit inference is unchanged. Effective-batch-size reporting fix originally identified by @0xxreacher in #905. Co-authored-by: 0xxreacher <0xxreacher@users.noreply.github.com>
The effective-batch-size warning fix from this PR landed on master via bee-san#921 (credited there). Take master's src/main.rs here; the remaining cleanups are re-applied in the next commit.
Re-applied on top of master (the batch-size warning fix already landed via bee-san#921): - Iterate over scripts_to_run by reference and clone only the script being run, instead of cloning the whole list for every IP. - Use HashMap entry or_default(). - If the soft fd limit does not fit in a usize (32-bit targets), fall back to DEFAULT_FILE_DESCRIPTORS_LIMIT instead of usize::MAX, which skipped every batch-size adjustment. - Fix the 'aveage' typo. Based on the original changes by @0xxreacher.
bee-san
approved these changes
Oct 1, 2026
bee-san
left a comment
Owner
There was a problem hiding this comment.
Thanks @0xxreacher! Your batch-size warning fix already landed through #921, where @DanHouseman credited you (you're also co-author on that commit). I merged master in and re-applied the rest of your cleanups on top:
scripts_to_runis iterated by reference and only the script being run is cloned. Before, the whole list was cloned for every IP.or_default()instead ofor_insert_with(Vec::new).- The
usize::MAXfallback inadjust_ulimit_sizeis nowDEFAULT_FILE_DESCRIPTORS_LIMIT. Good catch:usize::MAXwould have skipped every batch-size adjustment. - The 'aveage' typo is fixed.
I left out the FIX #n comments and the constant-only test, and I kept the Call format debug log because it's at a different log level from the output! line. CI is green on all four platforms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #1 - Wrong batch size in warning message (opts.batch_size → batch_size): The warning was showing the raw CLI value, not the actual adjusted value that ran. Users would try lowering a number that wasn't what the scanner used.
Fix #2 - or_insert_with(Vec::new) -> or_default(): Idiomatic Rust, clippy::pedantic flags the old form. Functionally identical but cleaner.
Fix #3 - scripts_to_run.clone() moved out of loop: Was cloning the entire Vec once per IP. Now iterates by reference (for script_f in &scripts_to_run) and only clones the individual ScriptFile when mutation is actually needed. Zero behavior change, much better perf at scale.
Fix #4 - Removed redundant debug!("Call format {call_f}"): The output! macro right above it already logs the full call format string. Duplicate log at a different level adds noise.
Fix #5 - usize::MAX fallback → DEFAULT_FILE_DESCRIPTORS_LIMIT: The most dangerous one. If try_into() ever failed, the returned usize::MAX would make infer_batch_size think the system has infinite file descriptors and skip all safety adjustments potentially crashing with a massive batch size. Now falls back to the same conservative 8000 limit the rest of the code uses.