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

Fix overrun when receiving debug packet (0xFF) in debug builds #219

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Fix overrun when receiving debug packet (0xFF) in debug builds #219

merged 1 commit into from
Apr 29, 2024

Conversation

twostars
Copy link
Contributor

We raise it to effectively support a total of 256 opcodes instead of 255, as 0~255 inclusive = 256 opcodes, causing it to overrun when sending/receiving the N3_TEMP_TEST packet.

Your checklist for this pull request

🚨Please review the guidelines for contributing to this repository.

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your master!
  • [ x Make sure you are making a pull request against the canary branch (left side). Also you should start your branch off our canary.
  • Check the commit's or even all commits' message styles matches our requested structure.
  • Check your code additions will fail neither code linting checks nor unit test.

Description

This fixes an overrun when receiving the debug packet (N3_TEMP_TEST / 0xFF) in debug builds.
We raise it to effectively support a total of 256 opcodes instead of 255, as 0~255 inclusive = 256 opcodes, which originally caused it to overrun when sending/receiving the N3_TEMP_TEST (0xFF) packet.

💔Thank you!

Copy link
Contributor

@UTengine UTengine left a comment

Choose a reason for hiding this comment

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

+1 (256) fixes the issue

@twostars
Copy link
Contributor Author

twostars commented Apr 29, 2024

+1 (256) fixes the issue

I am aware, since I was the one that debugged and fixed this for you. It felt more appropriate in this case to use UCHAR_MAX to represent the max potential opcode size, given they're unsigned bytes.

I would have moved the opcodes into an enum and done it that way, but didn't want any linting concerns, nor am I familiar with the expected naming conventions here. So I resorted to using UCHAR_MAX + 1 instead to more accurately represent what this is (max value of an unsigned byte -- our opcode -- +1 to include all of them), rather than the magic number that is 256.

src/game/APISocket.h Show resolved Hide resolved
We raise it to effectively support a total of 256 opcodes instead of 255,
as 0~255 inclusive = 256 opcodes, which originally caused it to overrun when
sending/receiving the N3_TEMP_TEST (0xFF) packet.
@stevewgr stevewgr merged commit 879a765 into ko4life-net:master Apr 29, 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