Skip to content

Remove Undefined Behavior in ISFS Example#28

Merged
DacoTaco merged 2 commits intodevkitPro:masterfrom
Geotale:master
Aug 4, 2025
Merged

Remove Undefined Behavior in ISFS Example#28
DacoTaco merged 2 commits intodevkitPro:masterfrom
Geotale:master

Conversation

@Geotale
Copy link
Copy Markdown
Contributor

@Geotale Geotale commented Aug 2, 2025

Undefined behavior was causing crashes in isfs.c, because specific values on the initial directory read from were expected to by 0 but had the chance to not be
Specifically, with the initial directory that gets read, the children pointer may not have been NULL, leading to AddChildEntry assuming there was already allocated data, and size could've been nonzero, leading it to assume the total number of children had already been requested (although likely with no effect on the example program itself)

Undefined behavior was causing crashes in isfs.c, because specific values were expected to by 0 but had the chance to not be
Comment thread filesystem/directory/isfs/source/main.c Outdated
@@ -357,6 +357,8 @@ int main(int argc, char** argv) {
DIR_ENTRY parent;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think a better fix would be to init the struct with zeroes. this can be done by doing DIR_ENTRY parent = {0} or DIR_ENTRY parent = {}. then you wouldn't need to go over all variables and set them to zero :)

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.

Sounds good to me ^^

Initializing the entire struct with 0 removes any possibility of *future* UB as well hopefully, or at least makes sure nothing was missed, while generally being more concise ^^;
Copy link
Copy Markdown
Member

@DacoTaco DacoTaco left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for your contribution!

@DacoTaco DacoTaco merged commit 6c95694 into devkitPro:master Aug 4, 2025
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