control: Avoid hangup in the recvdata path

[rsmarples]

Instead return an error and bubble it up where it can be hangup / freed more cleanly.

Reported by CuB3y0nd root@cubeyond.net

[coderabbitai]

Warning

Review limit reached

@rsmarples, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 12 minutes and 56 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3c50af2e-593f-483a-ba04-497ccecdd3d5

📥 Commits

Reviewing files that changed from the base of the PR and between c8435bc and d7ee0b2.

📒 Files selected for processing (1)

• src/control.c

Walkthrough

Refactors the dhcpcd control socket event loop to change control_recvdata from void to int, have control_handle_read return on EOF/error without directly hanging up the fd, consolidate cleanup in control_handle_data under a single goto hangup label, and update ps_ctl_dispatch to conditionally free the control client on failure.

Changes

Control read path error propagation and cleanup

Layer / File(s)	Summary
control_recvdata return type and internal error returns src/control.h, src/control.c	control_recvdata declaration changes from void to int; parsing failures (no terminator, no arg buffer) return -1; dhcpcd_handleargs failure returns -1 without calling control_free, shifting cleanup responsibility to callers.
control_handle_read EOF and PRIVSEP error propagation src/control.c	control_handle_read returns immediately on read() returning -1 or 0 without hanging up the fd; within the PRIVSEP branch, ps_ctl_sendargs failure now returns -1 instead of 0.
Caller-side cleanup in control_handle_data and ps_ctl_dispatch src/control.c, src/privsep-control.c	control_handle_data is restructured with a unified goto hangup path that calls control_hangup(fd) for write/read failures and ELE_HANGUP; ps_ctl_dispatch adds an err variable and conditionally frees the client fd and clears ctx->ps_control_client when control_recvdata returns -1 or 0.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 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 accurately describes the main change: modifying control.c to avoid calling hangup within the recvdata path, returning errors instead for upstream handling.
Description check	✅ Passed	The description is related to the changeset, explaining that errors are returned and propagated upstream for cleaner hangup/cleanup handling.
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 unit tests (beta)

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

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 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/control.c`:
- Around line 116-117: The conditional at lines 116-117 returns bytes unchanged
when bytes equals 0 (EOF), but the caller at line 211 only treats -1 as a hangup
signal, leaving the closed file descriptor registered and causing event loop
spinning. Modify the return statement to return -1 when bytes equals 0 to
properly signal EOF as a hangup condition, while preserving the existing PRIVSEP
semantics for other return paths.
- Around line 247-250: The buffer boundary check in the `argvp` array validation
does not account for the NULL terminator that is written later. The current
check allows argc to equal the full array size, but Line 267 writes a NULL
sentinel value which would exceed the buffer bounds. Modify the boundary check
condition to reserve one slot for the terminating NULL pointer by adjusting the
comparison to use `sizeof(argvp) / sizeof(argvp[0]) - 1` instead of the full
size, so that the check rejects input when argc would consume all available
slots including the one needed for the NULL terminator.

🪄 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: 3bc16174-c20d-4836-9d96-58b110fc5c20

📥 Commits

Reviewing files that changed from the base of the PR and between 7240d83 and 0789258.

📒 Files selected for processing (3)

• src/control.c
• src/control.h
• src/privsep-control.c