Conversation
WalkthroughAdded a new CLI option Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Neo - PR Security ReviewNo security issues found Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/mapcidr/main.go`:
- Line 124: The --range flag (options.Range) can be silently ignored because
process() only invokes commonFunc() on certain branches; add a compatibility
guard in validateOptions() to detect when options.Range is true together with
mutually incompatible flags (e.g., --aggregate, --shuffle-ip, --sort, --count)
and return an error (or non-zero exit) so the program fails fast; update
validateOptions() to list the conflicting flags, check their values, and surface
a clear error message referencing --range and the conflicting flag(s) so callers
know to remove the incompatible combination.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18c8da5f-bc36-4697-9054-416c98527b1f
📒 Files selected for processing (1)
cmd/mapcidr/main.go
| flagSet.BoolVarP(&options.Aggregate, "aggregate", "a", false, "Aggregate IPs/CIDRs into minimum subnet"), | ||
| flagSet.BoolVarP(&options.AggregateApprox, "aggregate-approx", "aa", false, "Aggregate sparse IPs/CIDRs into minimum approximated subnet"), | ||
| flagSet.BoolVarP(&options.Count, "count", "c", false, "Count number of IPs in given CIDR"), | ||
| flagSet.BoolVarP(&options.Range, "range", "r", false, "Convert CIDR to IP range (e.g. 192.168.0.0-192.168.255.255)"), |
There was a problem hiding this comment.
--range can be silently ignored with other processing flags.
process() only calls commonFunc() on specific paths; with flags like --aggregate, --shuffle-ip, --sort, --count, etc., --range won’t execute and no error is raised. Please fail fast on incompatible combinations in validateOptions().
Proposed guard in validateOptions()
func (options *Options) validateOptions() error {
@@
if options.ToIP4 && options.ToIP6 {
return errors.New("IP4 and IP6 can't be converted together")
}
+
+ if options.Range && (options.Aggregate || options.AggregateApprox || options.Shuffle ||
+ options.SortAscending || options.SortDescending || options.Count ||
+ options.Slices > 0 || options.HostCount > 0) {
+ return errors.New("range can't be used with aggregate/aggregate-approx/shuffle/sort/count/sbc/sbh")
+ }
+
if options.FilterIP != nil && options.MatchIP != nil {
return errors.New("both match and filter mode specified")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/mapcidr/main.go` at line 124, The --range flag (options.Range) can be
silently ignored because process() only invokes commonFunc() on certain
branches; add a compatibility guard in validateOptions() to detect when
options.Range is true together with mutually incompatible flags (e.g.,
--aggregate, --shuffle-ip, --sort, --count) and return an error (or non-zero
exit) so the program fails fast; update validateOptions() to list the
conflicting flags, check their values, and surface a clear error message
referencing --range and the conflicting flag(s) so callers know to remove the
incompatible combination.
2 participants
closes #623
Adds
-range/-rflag to convert CIDR to IP range.Summary by CodeRabbit
--range/-rCLI option to convert CIDR notation to IP address ranges infirstIP-lastIPformat.