refactor: one definition of is_pid_running - #37
Merged
Conversation
simd carried its own copy of is_pid_running alongside simapi's. Both were the same function, and simd already links simapi and includes simapi.h where it is declared -- the duplicate resolved only because an executable's own definition wins over the shared library's. That was survivable while the two were identical. It stopped being so when the function grew a second code path: sandboxed, the pids being tracked belong to the host's namespace where kill(2) cannot reach them, so the lookup routes through the host. A fix to that logic had to be made twice, in two files, and nothing would report it if only one were changed. simd's copy is removed and the call now resolves to the library's, which also picks up simd's better ordering -- reject a bogus pid before asking the host anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Spacefreak18
pushed a commit
that referenced
this pull request
Sep 10, 2026
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.
Responding to your maintainability point on #35 — you were right, and this is the sharpest instance of it.
is_pid_runningexisted twice: once insimapi/getpid.cand once insimd/simd.c. Both were the same function. The duplicate resolved only because an executable's own definition wins over the shared library's, even though simd links simapi and includessimapi.hwhere it is declared.Two identical copies were survivable. They stopped being so when the function grew a second code path — sandboxed, the pids being tracked belong to the host's namespace where
kill(2)cannot reach them, so the lookup routes through the host. That made it a function with two implementations and two locations: a fix had to be applied twice, and nothing would tell you if you only changed one.simd's copy is removed. The call resolves to the library's, confirmed by the symbols:
The surviving copy also takes simd's better ordering — reject a bogus pid before asking the host anything.
Builds and runs clean. This removes one of the two places the sandbox branching had to be maintained; the remaining seams are all single dispatch points rather than duplicated logic.
I'd also suggest testing the sandboxed path in CI, since nothing currently exercises it — the Flatpak job installs and runs the bundle, so asserting simd sees the host's process table rather than the sandbox's four would turn a silent rot into a build failure. Happy to add that separately if you want it.
🤖 Generated with Claude Code