Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LinuxEmulation: Ensure syscall wrapper declaration has CpuStateFrame as the first argument #4290

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

Sonicadvance1
Copy link
Member

@Sonicadvance1 Sonicadvance1 commented Jan 22, 2025

Otherwise crashes occur.

@Sonicadvance1 Sonicadvance1 changed the title LinuxEmulation: Oops, messed up the function signature on the new v6.… LinuxEmulation: Oops, messed up the function signature on the new v6.13 impls Jan 22, 2025
@neobrain
Copy link
Member

Oof, I assumed the syscall macros covered some minimal signature checking at least, but guess not...

Including external references in PRs can help prevent this, as it makes it easier for reviewers to cross-check. The author typically already knows where the reference is, so it would make sense to include it in the description.

Besides that, could you reword the commit message to something more factual and informative?

…as the first argument

Otherwise crashes occur.
@Sonicadvance1 Sonicadvance1 changed the title LinuxEmulation: Oops, messed up the function signature on the new v6.13 impls LinuxEmulation: Ensure syscall wrapper declaration has CpuStateFrame as the first argument Jan 23, 2025
@Sonicadvance1
Copy link
Member Author

Added more words to the commit message with more meaningful meanings.

Copy link
Member

@neobrain neobrain left a comment

Choose a reason for hiding this comment

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

Fixing an oversight in #4283 .

@Sonicadvance1 Sonicadvance1 merged commit 8d6a43d into FEX-Emu:main Jan 23, 2025
12 checks passed
@Sonicadvance1 Sonicadvance1 deleted the fix_v6.13 branch January 23, 2025 21:55
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