Remove dead code and simplify code introduced during sidecar migration - #284
Merged
Conversation
henrybear327
marked this pull request as draft
August 11, 2026 19:24
henrybear327
marked this pull request as ready for review
August 11, 2026 19:25
henrybear327
force-pushed
the
sysroot/redundant-work
branch
from
August 11, 2026 19:43
add8997 to
8162e84
Compare
henrybear327
marked this pull request as draft
August 11, 2026 20:08
henrybear327
marked this pull request as ready for review
August 11, 2026 21:50
![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
sysroot/redundant-work
branch
from
August 11, 2026 22:45
8162e84 to
ba5dacb
Compare
Collaborator
Author
|
#290 fixes the issue raised in https://github.com/sysprog21/elfuse/actions/runs/31543781893/job/93951954868?pr=284 |
henrybear327
marked this pull request as draft
August 11, 2026 23:14
henrybear327
force-pushed
the
sysroot/redundant-work
branch
2 times, most recently
from
August 13, 2026 12:14
9aef30d to
ff2fa48
Compare
henrybear327
marked this pull request as ready for review
August 13, 2026 12:14
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
henrybear327
marked this pull request as draft
August 13, 2026 12:27
Two computations ran on every translation that reached them, and nothing read their results. path_check_relative_sysroot_containment() filled a 4 KiB buffer with the reconstructed absolute guest path for callers that need it, but one caller passes NULL and the other never reads the buffer again. Both byte-exact resolver arms then re-concatenated the sysroot and the lookup into a buffer the seed had already spelled from the same operands, nothing on those arms having rebased the lookup. The two spellings differ in one case: under --sysroot /, the snprintf produced a doubled separator, the prefix and the clamped lookup each carrying a slash. Every consumer either passes the buffer through realpath() or special-cases the one-byte prefix, so no behavior depends on that extra byte.
Four things in the walk's per-component loop had no reader or no reachable caller. resolve_component() reported presence through @present beside its verdict, and the two never diverged: presence was set on exactly the paths that return PROBE_EXACT, so callers test the verdict instead. probe_exact() guarded its @is_link out-param against a NULL that no caller passes. The escape fallback in resolve_component() cannot run: path_component_copy() delivers a non-empty, slash-free name of at most CASEFOLD_GUEST_NAME_MAX bytes into a statically sized buffer that casefold.h proves large enough, and its retry could only repeat the failure it handled. That arm now returns PROBE_ERROR rather than guess a spelling. casefold_resolve_at() required a walk report, so four of its six callers declared a casefold_walk_t only to satisfy the signature. The report becomes optional; three of those callers pass NULL here, and the fourth keeps its struct because a later commit reads it.
casefold_utf16_units, and the UTF-8 validator standing behind it, have no production caller: the escape decision already covers ill-formed UTF-8 through its byte >= 0x80 rule, and the unit budgets are enforced by static assert. The host lane's budget check is the only consumer. Move both there. Inside the codec, that check compares the production arithmetic against itself. In the test the counter is a second implementation, so a codec change that breaks the unit budget shows up as a disagreement between the two instead of being mirrored into both sides at once.
proc_sysroot_casefold_enabled(), which backs every casefold_active() call, took sysroot_lock several times per translation to read a flag decided once at startup, and re-published to a forked child, before any vCPU thread issues a syscall. No other state is kept consistent with it, so the lock provided ordering that nothing requires. Make the flag _Atomic and state the published-once invariant where it is set.
Two per-call computations re-derived values fixed at configuration time. sysroot_seed_host_path() took the sysroot snapshot, and with it a mutex acquisition, before testing whether the path is even absolute, so every relative-path syscall performed a snapshot it then discarded; the absolute-path test now runs first. sysroot_path_is_contained() re-derived realpath(sysroot) although proc_set_sysroot() canonicalizes the sysroot once; it now copies that snapshot, and falls back to the per-call derivation only for a sysroot that did not resolve then. The flag recording which of those two cases holds is read without the lock, so it is _Atomic, matching the fold flag beside it.
Each component copied the whole accumulated prefix into a private 4 KiB buffer before appending its candidate, twice when the literal probe missed, which is the rescan the running length exists to avoid. Append into the output buffer and restore the terminator instead: the per-component cost drops from a copy of the prefix to a copy of the component bytes. A candidate that does not fit still reports ENAMETOOLONG, an escape never being shorter than the literal it stands for. The restore also runs when the append itself fails. The separator is written before the length check that rejects the candidate, so without the restore an over-long component would leave a trailing slash in the caller's buffer.
Every component probe requests ATTR_CMN_OBJTYPE, and the answer was discarded and then derived again: the NO_XDEV walker re-asked with fstatat per component, and the walk re-confirmed a resolved leaf with faccessat. Record what the probe established in the walk report and gate both calls on it. The recheck stays wherever the walk cannot answer: the readdir fallback, whose listing carries no type and so withdraws whatever the probe before it learned, and the byte-exact arm, which runs no walk. The surviving fstatat in the NO_XDEV walker now runs only where the walk left link-ness unknown, and the restructure carries its errno rule across unchanged: ENOENT is itself an answer, nothing there being able to be a link, while any other errno leaves link-ness undecided and fails the walk. Mount classification depends on that walk, so it must not cross a component it could not type.
Every successful in-sysroot lookup called realpath() on a path the walk had just spelled, to prove the result lands inside the sysroot. A walk that answered every component's object type has already proven it: no symlink can hide in a path where every component was typed, so the host spelling cannot resolve anywhere but under the prefix it was built from. Record that condition in the walk report as all_types_known and skip the re-derivation when it holds. It does not hold for the readdir fallback, whose listing carries no type, or for the byte-exact arm, which runs no walk, and the create resolver's parent keeps its unconditional check: a mkdir may have just materialized it. The host lane pins the flag from both sides as a regression guard: the behavior arrives correct, so the checks cannot be observed red against this commit, but deleting the fallback's withdrawal makes the listing-answered case fail, which is the wrong answer this guard exists to catch.
henrybear327
force-pushed
the
sysroot/redundant-work
branch
from
August 13, 2026 12:52
ff2fa48 to
2c1a26c
Compare
henrybear327
marked this pull request as ready for review
August 13, 2026 14:18
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.
Follow up improvement effort to improve code introduce in #256
Summary by cubic
Removes sidecar-era paths and cuts per-lookup work in casefold and sysroot resolution. Old: each component copied prefixes, re-derived
realpath()and object types, and rebuilt resolver outputs; new: probe components in place, reuseATTR_CMN_OBJTYPE, skip redundantrealpath()and concatenations, and treat fully typed walks as containment proofs.Probe components in place: append candidates to
outand restore its terminator.outdoubles as probe scratch; do not overlapoutwithguest_pathorbase_host_prefix.Reuse object types from the probe: record
leaf_type_known,leaf_is_link, andall_types_known. Runfstatat/faccessatonly when types are unknown.host_component_spellingexposes link-ness to the NO_XDEV walker.Skip the sysroot containment recheck for a fully typed walk; keep it for listing-answered components, dot components, and byte-exact paths.
Canonicalize the sysroot once and read
_Atomicfold/canonical flags. Avoid per-callrealpath()and skip sysroot snapshots for relative paths.Remove unread resolver outputs and duplicate sysroot+lookup concatenations; rely on the seed buffer.
Make the walk report optional in
casefold_resolve_at; pass NULL when only the verdict oroutis needed.Move the UTF-8/UTF-16 budget oracles into tests; drop
casefold_utf16_unitsfromcasefold.h.Fixes
readdirfallback so callers do not skip requiredfstatatchecks.getattrlistatstored-name replies; prevent an overlong append from leaving a trailing/.Written for commit 2c1a26c. Summary will update on new commits.