Skip to content

Fixed <stdio.h> file and dereferencing bugs#807

Draft
ZERICO2005 wants to merge 1 commit into
masterfrom
stdio_patch
Draft

Fixed <stdio.h> file and dereferencing bugs#807
ZERICO2005 wants to merge 1 commit into
masterfrom
stdio_patch

Conversation

@ZERICO2005

@ZERICO2005 ZERICO2005 commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixed the following issues:

[P2] clearerr indexes file streams off by one: src/libc/clearerr.c:13. fopen stores slot 1 at _file_streams[0], but clearerr uses _file_streams[stream->slot]; slot 5 writes past FOPEN_MAX.
[P2] fread(stdin, ...) reports full success after EOF/partial reads: src/libc/fread.c:20. The loop stops on EOF but returns count unconditionally instead of completed elements.
[P2] fwrite(stdout, size=0, ...) divides by zero: src/libc/fwrite.c:17. For zero-sized writes, the loop does nothing, then returns num / size.

Also fixed:

  • Dereferencing FILE * stream when it is NULL, stdin, stdout, or stderr.
  • return value for fseek and ftell

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread src/libc/include/stdio.h

size_t fwrite(const void *__restrict ptr, size_t size, size_t count, FILE *__restrict stream);

long int ftell(FILE *stream) __attribute__((__warn_unused_result__));

@ZERICO2005 ZERICO2005 Jun 19, 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.

long int is archaic, so I have replaced it with just plain long.

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread src/libc/clearerr.c
Comment on lines +14 to +16
uint8_t index = stream->slot - 1;
_file_streams[index].eof = 0;
_file_streams[index].err = 0;

@ZERICO2005 ZERICO2005 Jun 19, 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.

fixed off by one error, since stream->slot is 1-indexed instead of 0-indexed

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread src/libc/fclose.c

_file_streams[slot - 1].slot = 0;

int status = ti_Close(slot);

@ZERICO2005 ZERICO2005 Jun 19, 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.

Only closing the file in libc if and only if fileioc was able to successfully close the file

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread src/libc/feof.c
Comment on lines +5 to +8
if (stream == NULL || stream == stdin || stream == stdout || stream == stderr)
{
return 0;
}

@ZERICO2005 ZERICO2005 Jun 19, 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.

We cannot dereference stream if it is NULL, stdin, stdout, or stderr

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread src/libc/fgetc.c

if (c == EOF)
{
stream->eof = 1;

@ZERICO2005 ZERICO2005 Jun 19, 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.

I fixed things so stdin, stdout, and stderr are not dereferenced here

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread src/libc/fread.c
size_t bytes_read = 0;

for (; len > 0; len--)
for (; len --> 0;)

@ZERICO2005 ZERICO2005 Jun 19, 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.

This is a more common idiom for writing the decrementing count

@mateoconlechuga mateoconlechuga Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No it fucking isn't

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread src/libc/fseek.c
}

return ti_Seek((int)offset, origin, stream->slot);
return (ti_Seek((int)offset, origin, stream->slot) == EOF) ? -1 : 0;

@ZERICO2005 ZERICO2005 Jun 19, 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.

fseek returns 0 on success and non-zero otherwise

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread src/libc/ftell.c
// ti_Tell shouldn't return a value greater than OS_VAR_MAX_SIZE (65512) unless an error occurs
uint16_t ret = ti_Tell(stream->slot);
// Convert a result of UINT16_MAX to -1L, leaving other results the same
return ((uint16_t)ret + 1) - 1L;

@ZERICO2005 ZERICO2005 Jun 19, 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.

This had an integer promotion bug. (uint16_t)ret + 1 results in int, not uint16_t

ZERICO2005
ZERICO2005 commented Jun 19, 2026
View reviewed changes
Comment thread test/standalone/asprintf_fprintf/src/main.c
Comment on lines +575 to +577
C(fseek(file, 0, SEEK_END) == 0);
int file_size = ftell(file);
fseek(file, 0, SEEK_SET);
C(fseek(file, 0, SEEK_SET) == 0);

@ZERICO2005 ZERICO2005 Jun 19, 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.

Added some basic return value checks. Mostly important fseek since it returned the wrong value prior.

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

Reviewers

@mateoconlechuga mateoconlechuga mateoconlechuga left review comments

Assignees

No one assigned

Labels

libc

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ZERICO2005 @mateoconlechuga