Options: Preserve v6 `vendclass` CLI optarg when parsing enterprise number by nbromberg-dn · Pull Request #725 · NetworkConfiguration/dhcpcd

nbromberg-dn · 2026-09-08T09:52:03Z

The issue

When passing --vendclass '<EN> <DATA>' via CLI arguments, the packet itself is sent with empty data (only with the EN).

The vendclass parser split the enterprise number from its class data by writing a NUL into optarg.
The command-line options are applied more than once: first while processing global options, then again when configuring each interface.

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 EN for later passes.

Example

Calling

$ dhcpcd -6 --vendclass "1234 Hello" ...

Transmitted on the wire (dissected using tshark):

    Vendor Class
        Option: Vendor Class (16)
        Length: 6
        Enterprise ID: Linkage Software Inc. (1234)
        vendor-class-data:

Live-debugging with GDB confirms:

Breakpoint 2, parse_option (ctx=ctx@entry=0x7fffffffd978, ifname=ifname@entry=0x0, ifo=ifo@entry=0x56c940, opt=147, arg=0x56a6a0 "1234", ldop=ldop@entry=0x0, edop=0x0) at if-options.c:2198
2198			u = (uint32_t)strtou(arg, NULL, 0, 0, UINT32_MAX, &e);


(gdb) x/10bc arg
0x56a6a0:	49 '1'	50 '2'	51 '3'	52 '4'	0 '\000'	72 'H'	101 'e'	108 'l'
0x56a6a8:	108 'l'	111 'o'

(gdb) p arg
$1 = 0x56a6a0 "1234"

(gdb) p fp
$2 = 0x56a6a5 "Hello"

The proposed fix

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.
fp still advances via strskipwhite into the vendor-class portion - behavior matches the old in-place split without mutating shared optarg.

With this fix, calling the same dhcpcd command as above now trasmits:

    Vendor Class
        Option: Vendor Class (16)
        Length: 11
        Enterprise ID: Linkage Software Inc. (1234)
        vendor-class-data: Hello

And GDB showing:

(gdb) x/10bc arg
0x5824e0:	49 '1'	50 '2'	51 '3'	52 '4'	32 ' '	72 'H'	101 'e'	108 'l'
0x5824e8:	108 'l'	111 'o'
# The 5th byte is kept as whitespace rather than changed to \0 as before

(gdb) p bp
$1 = 0x5977a0 "1234"
(gdb) p arg
$2 = 0x5824e0 "1234 Hello"

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.

coderabbitai · 2026-09-08T09:52:34Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 10a308ca-84c6-43da-9b7b-1e8fff3965fd

📥 Commits

Reviewing files that changed from the base of the PR and between 42ca579 and 261851a.

📒 Files selected for processing (1)

src/if-options.c

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

Walkthrough

The vendor class option parser now copies the enterprise-number prefix before parsing it. This prevents modification of the original argument during later per-interface option replays.

Changes

Vendor class parsing

Layer / File(s)	Summary
Non-destructive enterprise-number parsing `src/if-options.c`	The parser copies the enterprise-number prefix into a temporary heap buffer, handles allocation failure, parses the value, and frees the buffer. The original argument remains unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 26185

Vendor-class arguments now retain their data while the enterprise number is parsed, so repeated interface configuration can encode and transmit the configured vendor class correctly. The change is ready to merge.

Suggested reviewers: rsmarples

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes preserving the v6 vendclass CLI argument during enterprise-number parsing, which is the main change.
Description check	✅ Passed	The description accurately explains the vendclass data loss, the cause, the proposed fix, and the resulting behavior.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.}

nbromberg-dn · 2026-09-08T09:53:44Z

-		if (fp)
-			*fp++ = '\0';
-		u = (uint32_t)strtou(arg, NULL, 0, 0, UINT32_MAX, &e);
+		bp = NULL;


bp may not be the best variable for this purpose, wdyt?

For code readability, bp seems to be used specifically to parsing bit flags.
You may want to add another pointer for this class of data.

fp for field pointer
bp for bit flag pointer
np for network pointer (addresses and such)
dp for byte data pointer?

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)

ColinMcInnes · 2026-09-08T16:11:10Z

-		if (fp)
-			*fp++ = '\0';
-		u = (uint32_t)strtou(arg, NULL, 0, 0, UINT32_MAX, &e);
+		bp = NULL;


For code readability, bp seems to be used specifically to parsing bit flags.
You may want to add another pointer for this class of data.

fp for field pointer
bp for bit flag pointer
np for network pointer (addresses and such)
dp for byte data pointer?

nbromberg-dn commented Sep 8, 2026

View reviewed changes

nbromberg-dn mentioned this pull request Sep 8, 2026

Preserve vendclass optarg when parsing enterprise number drivenets/dhcpcd#20

Open

ColinMcInnes suggested changes Sep 8, 2026

View reviewed changes

Navigation Menu

Uh oh!

Options: Preserve v6 `vendclass` CLI optarg when parsing enterprise number - #725

Options: Preserve v6 `vendclass` CLI optarg when parsing enterprise number #725
nbromberg-dn wants to merge 1 commit into
NetworkConfiguration:masterfrom
drivenets:nbromberg/fix-vendclass-cli-parsing

nbromberg-dn commented Sep 8, 2026

Uh oh!

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

nbromberg-dn Sep 8, 2026

Uh oh!

ColinMcInnes Sep 8, 2026

Uh oh!

ColinMcInnes Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nbromberg-dn commented Sep 8, 2026

The issue

Example

The proposed fix

Uh oh!

coderabbitai Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

nbromberg-dn Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

ColinMcInnes Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

ColinMcInnes Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading