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 64 bit crash on large gamedata files #2173

Merged
merged 1 commit into from
Jun 9, 2024

Conversation

bottiger1
Copy link
Contributor

As explained here, there was a crash here caused by parsing a large gamedata file on 64 bits (it doesn't crash on 32 bits). It was fixed by changing the line endings from windows to linux.

#2172

Increasing a buffer size fixes the issue so the line ending change isn't needed. I suspect there's probably some pointer math error that is avoided when the buffer is large enough.

@bottiger1 bottiger1 marked this pull request as ready for review June 2, 2024 00:21
@asherkin
Copy link
Member

asherkin commented Jun 2, 2024

I think what you're seeing there happens when a read chunk starts exactly on a double quote. Increasing the buffer size would paper over it for your specific input file only.

I think the correct fix is to keep the previous character in a variable for the escape character check, rather than attempting to read backwards from the pointer. The other easy option would be overlapping the reads.

@bottiger1 bottiger1 force-pushed the fixparser branch 8 times, most recently from 307dccc to e70a459 Compare June 5, 2024 08:30
@peace-maker
Copy link
Member

It looks like unrelated updates to amtl and sourcepawn submodules found their way in here. Is this intentional?

@bottiger1
Copy link
Contributor Author

no it's not intentional. I had to shuffle a lot of stuff around since my pull requests needed to make 64 bit not crash aren't getting merged and I have to update amtl and sourcemod as well.

@psychonic
Copy link
Member

Thanks

@psychonic psychonic merged commit b6d98c7 into alliedmodders:master Jun 9, 2024
4 checks passed
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.

4 participants