fix: close leaked fds in openpty fallback, add static assertions#24
Open
rmorgans wants to merge 3 commits intomobydeck:mainfrom
Open
fix: close leaked fds in openpty fallback, add static assertions#24rmorgans wants to merge 3 commits intomobydeck:mainfrom
rmorgans wants to merge 3 commits intomobydeck:mainfrom
Conversation
- openpty fallback now closes master fd on grantpt/unlockpt/ptsname failure instead of leaking it - Add _Static_assert for SCROLLBACK_SIZE power-of-two requirement and packet buffer fitting in uint8_t length field Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Zero passes the power-of-two bitmask check but is not a valid ring buffer size. Add SCROLLBACK_SIZE > 0 guard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rmorgans
added a commit
to rmorgans/atch
that referenced
this pull request
Mar 15, 2026
Cycles 50 sessions under ulimit -n 64 to verify that the openpty fallback properly closes fds on failure paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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
openpty()inmaster.c(used on platforms without<pty.h>) opens/dev/ptmxbut leaks the fd ifgrantpt(),unlockpt(), or slaveopen()fails. Fixed with a singlegoto failcleanup path._Static_assertforSCROLLBACK_SIZE: the ring buffer uses& (SCROLLBACK_SIZE - 1)for index wrapping, which requires a positive power of two. Now enforced at compile time.SCROLLBACK_SIZE == 0is also rejected._Static_assertforpkt.u.buf:struct packetusesunsigned char len— the buffer must fit in 255 bytes. Now enforced at compile time.What bugs these fix
SCROLLBACK_SIZEto a non-power-of-two or zero would silently corrupt the scrollback buffer. Now caught at compile time.struct winsizeever exceeded 255 bytes on a platform,pkt.lenwould silently truncate.Test results
202/203 pass (1 pre-existing: test 93). ASan+UBSan clean.