Refactor elfuse launch flag parsing code - #285
Merged
jserv merged 5 commits intoAug 13, 2026
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
henrybear327
force-pushed
the
oci/refactor_launch_flag_code
branch
from
August 12, 2026 11:16
177f059 to
90531be
Compare
The report_pass / report_fail / report_skip helpers and the Results: summary emitter are not Rosetta-specific: any standalone test script that wants its per-binary output to match the matrix runner's format needs exactly this file. Name the library for what it does rather than for its first consumer, and drop the Rosetta wording from its header and its internal variable so a new non-Rosetta user does not read as borrowing another suite's private helper. Mechanical beyond that: the eleven test-rosetta-*.sh scripts update their source line and shellcheck directive in place.
main() carried three hand-maintained copies of the usage line: the --help block, the unknown-option error, and the missing-elf-path error. They had already drifted; the missing-elf-path copy predates the GDB stub and never learned [--gdb PORT] [--gdb-stop-on-entry], so the most common error path advertised an incomplete flag set. Define the flag list once in ELFUSE_USAGE_BODY and expand it at two separators. ELFUSE_USAGE joins the groups with spaces for the two log_error sites, which need the flat form because log_impl stamps its timestamp and source location on the first line only and a wrapped string would print ragged under that prefix. ELFUSE_USAGE_WRAPPED breaks and indents the same groups for --help, which has no prefix. A flag added to the list now reaches all three sites. --help keeps a wrapped, aligned synopsis, re-flowed from five lines to three and still within 80 columns; the missing-elf-path error gains the GDB flags it had been missing. tests/test-usage-synopsis.sh compares the renderings against each other rather than sampling the flag list, since sampling is what let the copies drift unnoticed: it unwraps --help's block and requires it to equal the flat line both error paths print, and requires those two to match. Run against the parent commit it fails the width and single-line assertions, and against main it fails the flat-form comparison, reporting the missing GDB flags.
elfuse_launch owns guest bring-up: guest_bootstrap_prepare, the FUSE-temp unlink, the sysroot casefold probe, vCPU creation, GDB init/sync/wait, the run loop, gdb_stub_shutdown, the shim counter and syscall histogram dumps, and guest_destroy. main() retains the original CLI argv (proctitle rewriting), option parsing, sysroot provisioning, the shebang loop, host cwd, and the heap resource cleanup, and now hands off through launch_args_t so another launcher (the planned OCI run helper) can share one bring-up path. Bring-up failures unwind through a single fail label instead of repeating the guest_destroy-plus-unlink tail at every error site. The --gdb x86_64 refusal is enforced inside elfuse_launch, once bring-up has set g.is_rosetta; main() keeps its early static-ELF probe as a fast pre-bring-up reject, and the two sites share LAUNCH_GDB_X86_64_MSG (tests/test-rosetta-cli.sh pins the text) so their diagnostics cannot diverge. Enforcing the rule at the launch layer makes every caller inherit it, not only main()'s parser. Ownership of the FUSE-materialized temp ELF moves with the bring-up: elfuse_launch owns the unlink from the prepare call onward, and main() drops its claim before handing off, so main's shared goto unwind cannot double-unlink a path whose ownership has transferred. runtime_set_process_title moves ahead of the handoff: it touches only the original argv, which main() owns, so the title rewrite is a caller concern, not a bring-up step. The embedded shim blob include moves along with its only consumer.
main() unwound through three shapes: a fail/cleanup label pair that called cleanup_main_resources (a single-caller helper taking six parameters, all locals of main), inline frees in the sysroot-too-long branch, and bare `return 1` on every earlier error. Each new resource had to pick its unwind, and the inline branch had already drifted once. Declare and zero-initialize everything the unwind reads above the option loop, fold the helper's body into a single cleanup label, and route every error exit through it. Zero state makes each cleanup step a no-op, so an early goto is as safe as a late one. The label doubles as the success epilogue: elfuse_launch's return falls through with its exit code, and exit_code starts at 1 so no error site assigns it. --version, --help, and fork-child mode keep their direct returns: they are not error paths and own nothing the label frees. The coming launch-flag work adds three more heap resources to main(); with one label they join one list instead of choosing among three unwinds.
tests/lib/report.sh sources tests/lib/test-runner.sh, which defines a run_check taking <tool> <pattern> and resolving the tool name through test_tool_path. test-rosetta-statics.sh defined its own run_check taking <label> <regex> <expected-rc> <binary>, so sourcing the library and then defining the local helper left the library's version unreachable in this script. The definition order already selected the local helper, so nothing was misdispatched; the hazard was that moving the definition above the source line, or calling the library's run_check on the assumption it was the documented one, would silently pass absolute binary paths and an expected exit code to a function that reads them as a tool name and a regex. Rename the local helper to run_static_check and state at its header why the shorter name is unavailable. All 21 checks pass unchanged.
henrybear327
force-pushed
the
oci/refactor_launch_flag_code
branch
from
August 12, 2026 18:50
90531be to
35d2cfd
Compare
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
2 participants
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.
Summary by cubic
Extracts VM bring-up/run/teardown into
elfuse_launchand single-sources the usage synopsis to prevent drift. Error paths now share one cleanup path, and the missing-elf-path usage line includes the GDB flags it previously omitted.Refactors
elfuse_launch(const launch_args_t *args)insrc/core/launch.{c,h}; it owns bring-up, GDB init/sync/entry-stop, run loop, diagnostics, and teardown, and unlinks a FUSE-materialized temp ELF per the struct’s ownership contract.--gdbrejection for x86_64 (Rosetta) guests inside the launcher; shareLAUNCH_GDB_X86_64_MSGwithmain()’s early probe.ELFUSE_USAGE_BODY, rendered flat asELFUSE_USAGEfor error paths and wrapped asELFUSE_USAGE_WRAPPEDfor--help.main()to option parsing, sysroot provisioning, shebang handling, proctitle, and host cleanup; route errors through a singlecleanuplabel.core/launch.cto theMakefile.Tests
tests/test-usage-synopsis.shandmake test-usage-synopsisto compare wrapped--helpvs flat error-path synopses, require single-line errors, and assert 80-column width and alignment.tests/lib/rosetta-test.shtotests/lib/report.shand update alltests/test-rosetta-*.shscripts.tests/test-rosetta-statics.sh, rename the local helper torun_static_checkto avoid clashing with the library’srun_check.Written for commit 35d2cfd. Summary will update on new commits.