CDO Fixes For OpenBSD EXE Path

[samuelvenable]

TL;DR I'm CDO. It's like OCD, but in alphabetical order, just as it should be.

Summary by CodeRabbit

• Bug Fixes
  • Improved executable path resolution for OpenBSD systems to ensure more reliable identification and execution of the application across different environment configurations.

[coderabbitai]

Walkthrough

This PR refactors the OpenBSD-specific executable path resolution in OS_Unix::get_executable_path(). Helper lambdas encapsulate kvm and environment operations, argv retrieval is simplified to extract argv[0] as a single string, and the fallback strategy is restructured to attempt direct resolution, PATH-based search with retry, then PWD and getcwd fallbacks.

Changes

OpenBSD path resolution

Layer / File(s)	Summary
OpenBSD path resolution with kvm and fallback strategies drivers/unix/os_unix.cpp	Introduces cpp_getexe and cpp_getenv lambdas to encapsulate kvm and environment operations. Argv retrieval changes from vector accumulation to single cmd[0] string. Path resolution branches based on argv[0] content (presence of / or :) to choose between direct path resolution or PATH-based search, includes a single hardcoded PATH/HOME retry, then falls back to PWD and getcwd resolution.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Redot-Engine/redot-engine#1243: Both PRs modify drivers/unix/os_unix.cpp in OS_Unix::get_executable_path() for OpenBSD, reworking executable-path resolution using kvm-derived argv and multi-strategy fallback logic.

Suggested reviewers

• Arctis-Fireblight

🚥 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'CDO Fixes For OpenBSD EXE Path' directly relates to the main change: fixing OpenBSD executable path resolution in OS_Unix::get_executable_path().
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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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

🧹 Nitpick comments (1)

drivers/unix/os_unix.cpp (1)

1210-1262: ⚖️ Poor tradeoff

Goto-driven control flow is functional but hard to follow; consider documenting the retry path.

The dispatch around fallback: / retry: and the comparison slash_pos > colon_pos (which silently relies on std::string::npos being the max size_t so the branch only fires when the colon precedes the slash or there is no slash at all) is correct but non-obvious. A short comment above the else if and above retry: describing the cases each handles (absolute path / bare name or colon-prefixed / PATH retry with hard-coded defaults) would materially help future maintenance without changing behavior.

🤖 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 `@drivers/unix/os_unix.cpp` around lines 1210 - 1262, Add brief explanatory
comments to clarify the goto-driven control flow around the fallback and retry
labels: above the "else if (slash_pos == std::string::npos || slash_pos >
colon_pos)" branch document that this handles bare executable names or cases
where the colon precedes the slash (search PATH entries and handle
"name:subname" variants), and above the "retry:" label document that this loop
iterates PATH entries and that the retried=true block supplies a hard-coded
fallback PATH plus $HOME/bin before re-entering the retry loop. Reference the
existing symbols (fallback:, retry:, slash_pos, colon_pos, cpp_getexe,
cpp_getenv, retried) so maintainers understand which cases each comment
describes without changing logic.

🤖 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 `@drivers/unix/os_unix.cpp`:
- Around line 1164-1184: The loop currently stops early because its condition
uses kif[i].fd_fd < 0, which assumes KERN_FILE_TEXT entries come before regular
fds; change the loop to iterate over all entries (for (int i = 0; i < cntp;
i++)) and keep only the inner check if (kif[i].fd_fd == KERN_FILE_TEXT) to
detect the text file, preserving the existing fallback/error/res logic
(including the fallback: label) and only breaking once the KERN_FILE_TEXT
handling completes successfully.

---

Nitpick comments:
In `@drivers/unix/os_unix.cpp`:
- Around line 1210-1262: Add brief explanatory comments to clarify the
goto-driven control flow around the fallback and retry labels: above the "else
if (slash_pos == std::string::npos || slash_pos > colon_pos)" branch document
that this handles bare executable names or cases where the colon precedes the
slash (search PATH entries and handle "name:subname" variants), and above the
"retry:" label document that this loop iterates PATH entries and that the
retried=true block supplies a hard-coded fallback PATH plus $HOME/bin before
re-entering the retry loop. Reference the existing symbols (fallback:, retry:,
slash_pos, colon_pos, cpp_getexe, cpp_getenv, retried) so maintainers understand
which cases each comment describes without changing logic.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ecaae307-ec94-4db4-8c8e-0712476461a0

📥 Commits

Reviewing files that changed from the base of the PR and between 793bbca and d30c229.

📒 Files selected for processing (1)

• drivers/unix/os_unix.cpp