Skip to content

Conversation

@DrXiao
Copy link
Collaborator

@DrXiao DrXiao commented Apr 7, 2025

According to the man page, the return value of fputc() should be either the character written as an unsigned char cast to an int or EOF on error, but the current implementation doesn't follow the specification.

Therefore, the proposed changes correct the implementation to return the ASCII code of the written character or -1 if the output fails, thereby matching the description in the man page.

Additionally, the necessary test cases are also added to validate the correctness.

Summary by Bito

This pull request corrects the fputc function implementation to return the ASCII code of the written character or -1 on error, as per specifications. It also adds comprehensive test cases to validate the function's behavior, improving reliability and testing coverage.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 2

@DrXiao DrXiao force-pushed the libc/fix-fputc branch 4 times, most recently from 47bc037 to 924d629 Compare April 8, 2025 13:49
Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Rebase the latest master branch and resolve the conflicts.

According to the man page, the return value of fputc() should be either
the character written as an "unsigned char" cast to an "int" or EOF on
error, but the current implementation doesn't follow the specification.

Therefore, these changes correct the implementation to return the ASCII
code of the written character or -1 if the output fails, thereby
matching the description in the man page.

Additionally, the necessary test cases are also added to validate the
correctness.
@jserv jserv merged commit fcfe649 into sysprog21:master Apr 9, 2025
6 checks passed
@jserv
Copy link
Collaborator

jserv commented Apr 9, 2025

Thank @DrXiao for contributing!

@DrXiao DrXiao deleted the libc/fix-fputc branch April 9, 2025 13:43
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