Skip to content

fix: narrow REAPED_STATUSES cfg to non-Linux unix only#346

Merged
jdx merged 1 commit into
mainfrom
fix/reaped-statuses-dead-code
Apr 11, 2026
Merged

fix: narrow REAPED_STATUSES cfg to non-Linux unix only#346
jdx merged 1 commit into
mainfrom
fix/reaped-statuses-dead-code

Conversation

@jdx

@jdx jdx commented Apr 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • REAPED_STATUSES was declared with #[cfg(unix)] but only written/read behind #[cfg(all(unix, not(target_os = "linux")))]
  • On Linux CI, this caused a dead_code warning that fails clippy -D warnings
  • Narrowed the declaration's cfg gate to match its actual usage

Test plan

  • cargo clippy --all-targets --all-features -- -D warnings passes on Linux without warnings

🤖 Generated with Claude Code


Note

Low Risk
Low risk: this only tightens a cfg gate for a static used exclusively on non-Linux Unix, reducing Linux-only dead_code/clippy warnings without changing runtime behavior on supported paths.

Overview
Narrowed the REAPED_STATUSES static’s compile-time cfg from #[cfg(unix)] to #[cfg(all(unix, not(target_os = "linux")))] so it is only compiled on platforms where it’s actually used.

This avoids Linux dead_code warnings (and resulting clippy -D warnings failures) while leaving Linux zombie-reaping behavior unchanged.

Reviewed by Cursor Bugbot for commit 7d53370. Bugbot is set up for automated code reviews on this repo. Configure here.

The static is only written/read behind #[cfg(all(unix, not(target_os =
"linux")))], but was declared with #[cfg(unix)], causing a dead_code
warning on Linux.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a dead-code warning on Linux CI by narrowing the #[cfg] gate on REAPED_STATUSES from #[cfg(unix)] to #[cfg(all(unix, not(target_os = "linux")))], matching the gates already present on every write and read site. The fix is minimal and surgical with no behavioral change.

Confidence Score: 5/5

Safe to merge — one-line cfg gate correction with no behavioral change.

The change is a targeted, correct fix that aligns the declaration's cfg gate with every existing usage site. No logic is altered, and the fix resolves a real clippy -D warnings failure on Linux CI.

No files require special attention.

Important Files Changed

Filename Overview
src/supervisor/mod.rs Narrows REAPED_STATUSES declaration cfg gate to #[cfg(all(unix, not(target_os = "linux")))], exactly matching all read/write sites — eliminates Linux dead_code warning with no logic change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SIGCHLD received] --> B{Platform?}
    B -->|Linux| C["waitid(Id::All, WNOWAIT|WNOHANG|WEXITED)\nPeek at next zombie"]
    B -->|non-Linux Unix| D["waitpid(None, WNOHANG)\nBlind reap — may race"]

    C --> E{PID managed?}
    E -->|Yes| F[Skip — leave for Tokio child.wait]
    E -->|No| G["waitpid(Pid, WNOHANG)\nActually reap it"]

    D --> H{PID managed?}
    H -->|No| I[Reap as orphan]
    H -->|Yes – race lost| J["REAPED_STATUSES.insert(pid, code)\n#[cfg(all(unix, not(target_os = 'linux')))]"]
Loading

Reviews (1): Last reviewed commit: "fix: narrow REAPED_STATUSES cfg to non-L..." | Re-trigger Greptile

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the conditional compilation for the REAPED_STATUSES static variable in src/supervisor/mod.rs. The variable is now restricted to non-Linux Unix platforms, as Linux utilizes waitid with WNOWAIT to handle process reaping without needing this map. I have no feedback to provide.

@jdx jdx merged commit b3a5662 into main Apr 11, 2026
6 checks passed
@jdx jdx deleted the fix/reaped-statuses-dead-code branch April 11, 2026 13:17
@jdx jdx mentioned this pull request Apr 11, 2026
jdx added a commit that referenced this pull request Apr 12, 2026
## 🤖 New release

* `pitchfork-cli`: 2.5.0 -> 2.6.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [2.6.0](v2.5.0...v2.6.0) -
2026-04-12

### Added

- *(proxy)* auto start when visiting the proxied URL
([#347](#347))

### Fixed

- some issues related to sudo supervisor
([#323](#323))
- *(port)* should fail when ready_port is in use
([#350](#350))
- *(deps)* update rcgen to 0.14
([#349](#349))
- *(deps)* update reqwest to 0.13
([#348](#348))
- detect port conflicts on loopback addresses, not just 0.0.0.0
([#345](#345))
- narrow REAPED_STATUSES cfg to non-Linux unix only
([#346](#346))
- *(deps)* update rust crate ratatui to 0.30
([#331](#331))
- *(deps)* update rust crate toml to v1
([#344](#344))
- *(deps)* update rust crate strum to 0.28
([#334](#334))
- *(deps)* update rust crate notify-debouncer-full to 0.7
([#330](#330))
- *(deps)* update rust crate nix to 0.31
([#329](#329))
- *(deps)* update rust crate listeners to 0.5
([#328](#328))
- *(deps)* update rust crate sysinfo to 0.38
([#335](#335))
- *(deps)* update rust crate cron to 0.16
([#324](#324))
- *(deps)* update rust crate crossterm to 0.29
([#325](#325))

### Other

- *(deps)* update rust crate rmcp to v1.4.0
([#327](#327))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Low risk: this PR only bumps the crate version and updates release
notes, with no runtime code changes.
> 
> **Overview**
> Prepares the `pitchfork-cli` **v2.6.0** release by bumping the package
version from `2.5.0` to `2.6.0` in `Cargo.toml`/`Cargo.lock`.
> 
> Updates `CHANGELOG.md` with the `2.6.0` release notes (proxy
auto-start behavior, several fixes, and dependency updates).
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
faea6c5. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant