Skip to content

remove out of bounds array access#4751

Merged
Malkierian merged 1 commit intoHarbourMasters:developfrom
serprex:fix-oob
Dec 23, 2024
Merged

remove out of bounds array access#4751
Malkierian merged 1 commit intoHarbourMasters:developfrom
serprex:fix-oob

Conversation

@serprex
Copy link
Contributor

@serprex serprex commented Dec 22, 2024

Copy link
Contributor

@garrettjoecox garrettjoecox left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Typically what I'll do in these cases is check decomp repo to see if they have this bug documented, and submit a PR to document it with a comment, then add the comment in our repo stating we opted to fix it. (Example here)

You don't have to submit this to decomp if you don't want but it would be appreciated, at the very least we want it documented in our repo.

@serprex
Copy link
Contributor Author

serprex commented Dec 22, 2024

https://github.com/zeldaret/oot/blob/main/src/overlays/actors/ovl_En_Dy_Extra/z_en_dy_extra.c#L93

Looks like upstream opted to put 0x00 at end of array (causing same behavior)

I'll update PR to match upstream code

Fixed upstream in zeldaret/oot#1239

Copy link
Contributor

@garrettjoecox garrettjoecox left a comment

Choose a reason for hiding this comment

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

Looks like upstream opted to put 0x00 at end of array (causing same behavior)

I'll update PR to match upstream code

Even better! Looks like you missed a comma but otherwise looks good

@Malkierian Malkierian merged commit 781bbb8 into HarbourMasters:develop Dec 23, 2024
Pepper0ni pushed a commit to Pepper0ni/Shipwright that referenced this pull request Dec 23, 2024
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.

3 participants