Skip to content

DHCP: Add parentheses to macro definitions#585

Merged
rsmarples merged 1 commit intoNetworkConfiguration:masterfrom
acst1223:macro
Feb 26, 2026
Merged

DHCP: Add parentheses to macro definitions#585
rsmarples merged 1 commit intoNetworkConfiguration:masterfrom
acst1223:macro

Conversation

@acst1223
Copy link
Contributor

Missing parentheses in IP_UDP_SIZE caused wrong computation of MSZ (option57). When mtu is 1500, current MSZ value is 1488=1500-20+8, while the correct value should be 1472=1500-(20+8).

Also added parentheses for BOOTP_MIN_MTU.

Missing parentheses in IP_UDP_SIZE caused wrong computation of MSZ
(option57). When mtu is 1500, current MSZ value is 1488=1500-20+8, while
the correct value should be 1472=1500-(20+8).

Also added parentheses for BOOTP_MIN_MTU.
@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 661cdbd and f456059.

📒 Files selected for processing (1)
  • src/dhcp.c

Walkthrough

Macro definitions for DHCP size calculations are updated with explicit parenthesization to enforce correct operator precedence. IP_UDP_SIZE and BOOTP_MIN_MTU now use grouped expressions instead of relying on implicit precedence, eliminating potential miscalculation risks.

Changes

Cohort / File(s) Summary
DHCP Macro Precedence
src/dhcp.c
IP_UDP_SIZE and BOOTP_MIN_MTU macros wrapped in parentheses to enforce correct precedence in size calculations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding parentheses to macro definitions to fix operator precedence issues in DHCP size calculations.
Description check ✅ Passed The description is directly related to the changeset, explaining the problem (missing parentheses causing incorrect MSZ computation) and the solution implemented.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rsmarples
Copy link
Member

Looks good, thanks!

@rsmarples rsmarples merged commit f48f695 into NetworkConfiguration:master Feb 26, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants