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 "#pragma option" crash when character count is over 31 #462

Merged
merged 3 commits into from
Nov 7, 2019

Conversation

AmyrAhmady
Copy link
Contributor

@AmyrAhmady AmyrAhmady commented Oct 7, 2019

What this PR does / why we need it:

  • Title says it all!

Which issue(s) this PR fixes:

Fixes #359

What kind of pull this is:

  • A Bug Fix
  • A New Feature
  • Some repository meta (documentation, etc)
  • Other

Additional Documentation:

For some reason i will be 32 after our for loop so an OOB happens when code tries to assign '\0' to name[32] since name size is 32.
Doing a -1 to i fixed this issue and now throws

error 038: extra characters on line

when character count is over 31.

Also I'm not sure if I should change name size and make it able to accept more than 31 characters, since file paths can be more than 31 characters of course

For some reason `i` will be `32` after our for loop so an OOB happens when code tries to assign `'\0'` to `name[32]` since `name` size is 32.
Doing a `-1` to `i` fixed this issue and now throws `error 038: extra characters on line` when character count is over 31.
@AmyrAhmady AmyrAhmady requested a review from a team as a code owner October 7, 2019 07:52
Copy link
Member

@Y-Less Y-Less left a comment

Choose a reason for hiding this comment

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

This will fail for anything not 31 characters. For example #pragma option -O0 will become -O because you remove the second-to-last character. You need to check that i actually IS 32 (or rather sNAMEMAX+1).

@AmyrAhmady AmyrAhmady requested a review from Y-Less October 7, 2019 12:13
@Y-Less
Copy link
Member

Y-Less commented Oct 7, 2019

Simpler fix: Instead of i<sizeof name use i<sNAMEMAX.

@Y-Less Y-Less merged commit e7d757f into pawn-lang:dev Nov 7, 2019
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