You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Replace hardcoded magic multiplier with named constant
*The hardcoded 4.0 multiplier silently alters the defrag memcap calculation with no explanation in the code. Extract it into a named constant (e.g., DEFRAG_MEMCAP_ADJUSTMENT) with a comment referencing the hotfix noted in ISSUES.md, so the rationale is discoverable and the value can be corrected in one place.
- hashsize*DEFRAG_TRACKER_HASHROW+(max_defrag_tracker_active+(self.get_ippair_host_defrag_stream_reassembly_prealloc(answers, &HashType::Defrag) as f64)*MULTIPLIER)*DEFRAG_TRACKER*4.0+ /// Hotfix multiplier for defrag memcap estimation; see ISSUES.md "Hotfix for defrag_memcap".+ const DEFRAG_MEMCAP_ADJUSTMENT: f64 = 4.0;+ hashsize*DEFRAG_TRACKER_HASHROW+(max_defrag_tracker_active+(self.get_ippair_host_defrag_stream_reassembly_prealloc(answers, &HashType::Defrag) as f64)*MULTIPLIER)*DEFRAG_TRACKER*DEFRAG_MEMCAP_ADJUSTMENT+
Suggestion importance[1-10]: 4
__
Why: Extracting the unexplained *4.0 into a named constant with a reference to the ISSUES.md hotfix is a valid maintainability improvement for a change that silently alters the memcap calculation. However, it is purely stylistic and includes a doc comment, so the impact is moderate at best.
Low
Reduce excessive process kill timeout
Increasing the kill timeout from 30s to 300s means Suricata shutdown failures can now stall the whole workflow for 5 minutes. This looks like a leftover debugging value; a 10x increase is rarely justified for a process kill. Consider reverting to a more moderate timeout or making it configurable.
pub fn kill_suricata(child: &mut Child) {
- let end_timeout = Duration::from_secs(300);+ let end_timeout = Duration::from_secs(60);
Suggestion importance[1-10]: 2
__
Why: The PR deliberately increases end_timeout from 30s to 300s, likely an intentional change for slow Suricata shutdowns. The suggestion speculates it is a leftover debugging value without evidence and proposes an arbitrary 60s alternative, offering only marginal improvement.
Low
Add missing trailing newline to config
The file still lacks a trailing newline, which produces noisy diffs and can trigger "no newline at end of file" warnings in editors and linters. Add a final newline after the max_cpu_usage_vec line to conform to standard text file conventions.
Why: The diff does not indicate that the file lacks a trailing newline, and the improved_code is effectively identical to existing_code, making the change unverifiable and trivially impactful.
Low
Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.
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
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.
PR Type
Bug fix
Description
Parse CPU ranges in
max_cpu_usage_vec(e.g.0-2)Apply 4x correction factor to defrag memcap
Increase Suricata shutdown timeout to 300s
Bump version to 1.1.0-dev.2, update docs
Diagram Walkthrough
File Walkthrough
3 files
Parse CPU ranges in max_cpu_usage_vecApply 4x correction factor to defrag memcapIncrease Suricata shutdown timeout to 300s1 files
Make CPU range parsing helpers public2 files
Bump version to 1.1.0-dev.2Bump version and use CPU range example2 files
Update known issues listUpdate version references to 1.1.0-dev.2