Skip to content

Fix fflush(NULL) to flush all output streams - #266

Merged
tyfkda merged 1 commit into
tyfkda:mainfrom
emillaine:fflush-null
Oct 2, 2026
Merged

tyfkda merged 1 commit into
tyfkda:mainfrom
emillaine:fflush-null

Conversation

@emillaine

@emillaine emillaine commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Per C11 7.21.5.2p2, fflush(NULL) must flush all output streams. Previously it dereferenced NULL and segfaulted. Now it flushes stdout/stderr explicitly (statically allocated, not tracked in __fileman), returning EOF if either fails, and reuses the exit-time destructor _flush_opened_files for the remaining tracked files. Includes a fflush_null regression test.

@tyfkda tyfkda left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for fixing the missing part of the specification.

Could you also add a leading underscore to the flush_opened_files function name, remove the static keyword, and make sure the destructor has a void return type?

Comment thread libsrc/stdio/fflush.c Outdated
result = EOF;
if ((stderr->flush)(stderr) != 0)
result = EOF;
for (int i = 0, len = __fileman.length; i < len; ++i) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There is already a function in _fileman.c that performs similar operations. Could you modify this to call that function instead?

@emillaine emillaine Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review, I hadn't noticed that function. I made the requested changes and also added a test case. Is it now implemented as you intended?

@tyfkda tyfkda left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sorry for the minor nitpick, but could you move the function prototype from _fileman.h to _file.h?

@emillaine

Copy link
Copy Markdown
Contributor Author

No problem, done!

Per C11 7.21.5.2p2; previously it segfaulted on NULL.
@tyfkda
tyfkda merged commit 9eea241 into tyfkda:main Oct 2, 2026
1 check passed
@tyfkda

tyfkda commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Thanks!

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.

2 participants