vsio: Allow zero length options

[rsmarples]

The spec allows it and this is an oversight.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 414a8c91-1636-475d-b46f-b19b7b65cdc7

📥 Commits

Reviewing files that changed from the base of the PR and between 478bafe and c18688c.

📒 Files selected for processing (1)

• src/if-options.c

🚧 Files skipped from review as they are similar to previous changes (1)

• src/if-options.c

---

Walkthrough

In src/if-options.c, the O_VSIO/O_VSIO6 branch of parse_option is loosened to accept a vendorcode argument with no trailing ,option portion. The comma search and remainder pointer are made conditional, a zero-length payload now sets np = NULL directly, skipping malloc and parse_string, and the validation error message is corrected to log the argument instead of the file pointer.

Changes

O_VSIO vendor option parsing relaxation

Layer / File(s)	Summary
Tolerate missing/empty option, optimize payload, fix error message src/if-options.c	The comma search is made conditional: when absent or when the portion after the comma is empty, the remainder pointer is set to NULL and validation still proceeds on the numeric vendor option range. In the payload step, dl == 0 now sets np = NULL directly, bypassing malloc(0) and parse_string. The vsio is too big error message is corrected to log arg rather than fp.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• NetworkConfiguration/dhcpcd#578: Modifies the same O_VSIO/O_VSIO6 vendor-option parsing block in src/if-options.c, addressing adjacent NULL/empty argument handling issues in parse_option.

🚥 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%.	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 'vsio: Allow zero length options' directly and specifically summarizes the main change—enabling zero-length options in the vsio component as required by the specification.
Description check	✅ Passed	The description 'The spec allows it and this is an oversight' is directly related to the changeset, explaining the rationale for allowing zero-length options in compliance with the DHCP specification.
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.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch viso

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/if-options.c`:
- Around line 962-965: The variable `fp` can be set to NULL in the
vendorcode-only path (when the else branch executes), but it is later used in a
format string with the %s specifier in the size-error logging path, which causes
undefined behavior. Add a NULL guard before using `fp` in any logging statements
that include it as a format string argument. Replace the NULL `fp` value with a
safe string literal (such as an empty string or a descriptive placeholder) in
the log message to prevent crashes when the format string is processed.

🪄 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: 421d4792-42f3-462c-89b6-0ee4e15000e3

📥 Commits

Reviewing files that changed from the base of the PR and between b746a5b and 478bafe.

📒 Files selected for processing (1)

• src/if-options.c

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard %s logging against fp == NULL in vendorcode-only path.

After allowing missing payloads, fp can be NULL, but it is later formatted with %s in the size-error path. That introduces undefined behavior and can crash on some libc/log implementations.

Suggested fix

-		if (opt_header + dl > opt_max) {
-			logerrx("vsio is too big: %s", fp);
+		if (opt_header + dl > opt_max) {
+			logerrx("vsio is too big: %s", fp != NULL ? fp : "");
 			free(np);
 			return -1;
 		}

Also applies to: 1031-1033

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/if-options.c` around lines 962 - 965, The variable `fp` can be set to
NULL in the vendorcode-only path (when the else branch executes), but it is
later used in a format string with the %s specifier in the size-error logging
path, which causes undefined behavior. Add a NULL guard before using `fp` in any
logging statements that include it as a format string argument. Replace the NULL
`fp` value with a safe string literal (such as an empty string or a descriptive
placeholder) in the log message to prevent crashes when the format string is
processed.