Preserve vendclass optarg when parsing enterprise number - #20
Open
nbromberg-dn wants to merge 2 commits into
Open
Preserve vendclass optarg when parsing enterprise number#20nbromberg-dn wants to merge 2 commits into
nbromberg-dn wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The reviewed change resolves argument mutation with no unresolved issues.
Pull request overview
Preserves vendclass arguments across repeated command-line parsing.
Changes:
- Parses the enterprise number from a temporary copy.
- Avoids mutating shared
argvdata.
File summaries
| File | Description |
|---|---|
src/if-options.c |
Safely parses vendclass without altering optarg. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
nbromberg-dn
requested review from
a team,
aminescu-dn,
ekazula-dn,
graifen-dn,
mburlacu-dn,
mschreiber-dn and
ronharel02
September 6, 2026 10:38
mburlacu-dn
approved these changes
Sep 7, 2026
The vendclass parser split the enterprise number from its class data by writing a NUL into optarg. dhcpcd applies command-line options more than once: first while processing global options, then again when each interface is configured. That made the first pass succeed because it kept a local pointer to the bytes after the separator, but it permanently shortened the shared argv string to the enterprise number for later passes. Parse the enterprise number from a temporary NUL-terminated copy instead, leaving argv intact so the per-interface pass can still see and encode the vendor-class data. (cherry picked from commit 261851a from NetworkConfiguration#725)
nbromberg-dn
force-pushed
the
nbromberg/v26_4/SW-222918
branch
from
September 8, 2026 10:09
81498e4 to
a3c67b9
Compare
ronharel02
approved these changes
Sep 8, 2026
ColinMcInnes
suggested changes
Sep 8, 2026
ColinMcInnes
left a comment
ColinMcInnes
left a comment
There was a problem hiding this comment.
Split the README change out into its own PR, it's unrelated to the vendclass optarg issue.
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.
The
vendclassparser split the enterprise number from its class data by writing aNULintooptarg.dhcpcd applies command-line options more than once: first while processing global options, then again when each interface is configured.
That made the first pass succeed because it kept a local pointer to the bytes after the separator, but it permanently shortened the shared
argvstring to the enterprise number for later passes.Parse the enterprise number from a temporary NUL-terminated copy instead, leaving
argvintact so the per-interface pass can still see and encode the vendor-class data.vendclassCLI optarg when parsing enterprise number NetworkConfiguration/dhcpcd#725