Skip to content

fix(libc): close streams on invalid freopen modes - #5385

Closed
MarcZX10 wants to merge 3 commits into
boykopovar:mainfrom
MarcZX10:marc/freopen-invalid-mode-5379
Closed

MarcZX10 wants to merge 3 commits into
boykopovar:mainfrom
MarcZX10:marc/freopen-invalid-mode-5379

Conversation

@MarcZX10

Copy link
Copy Markdown
Contributor

What

Carries the three reviewed commits from #5379, preserving Hussein Kadhim’s commit authorship. An invalid mode now closes and frees a guest-owned stream before returning EINVAL; a previously closed stream also returns EINVAL without throwing. The standard streams remain open and usable, with the documented FreeBSD difference recorded in docs/dev/debt/libc.md.

The branch is rebased on current main. Its freopen changes integrate cleanly with open PR #5033, which adds Windows delete sharing during successful reopen.

Tested

Linux GCC 12.2, Ninja Release: built guest_standard_stream_tests and file_streams_tests; CTest guest_standard_streams and file_streams passed (2/2). The issue author’s Windows invalid-mode, descriptor-leak, and valid-mode comparison checks are summarized in #5379 and were not rerun here.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw; no new unimplemented path
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source; no third-party code added
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes; reviewed and tested by MarcZX100

FreeBSD closes the stream and keeps errno at EINVAL when freopen cannot parse the mode, with or without a path. The guest stream stayed open and the descriptor leaked.
freopen with an invalid mode threw on a stream that was already closed instead of returning null with EINVAL. The stream is now closed only once and the mode-only reopening limit is listed in the technical debt.
…d mode

Only streams the guest opened itself are closed. stdin, stdout and stderr stay open and usable, and the difference from FreeBSD is listed in the technical debt.
@likecrank-dotcom

Copy link
Copy Markdown

Independent Windows validation by OpenAI Codex (AI-assisted); no code changes were made to this PR.

On PR head 229d5280b31db08a9d0537ce4161ddad324f9190, I built guest_standard_stream_tests and file_streams_tests with Windows 11 Pro, WinLibs MinGW GCC 15.2.0, and CMake 4.4.4.

  • CTest guest_standard_streams: passed 5/5 consecutive runs.
  • CTest file_streams: passed 5/5 consecutive runs.
  • tools/check_conventions.py --base origin/main --head HEAD and git diff --check origin/main...HEAD: passed.

This was a focused Windows run of the two libc test targets, not a full build or full CTest suite.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the independent Windows run and repeated CTest results. That closes the platform-specific validation gap for the carried freopen fix; no code changes were needed.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: no title is known to reach this. With the queue at 700+ open PRs, please hold off on new PRs that don't name a title needing the change until it's down. If a title stops on this on current main, reopen with the title ID and the log line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants