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

AGS: Change a few char variables to int8 #3356

Merged
merged 1 commit into from Sep 10, 2021

Conversation

@antoniou79
Copy link
Contributor

@antoniou79 antoniou79 commented Sep 9, 2021

This is done to explicitly specify that they should be treated as signed

Some ports may treat an (unspecified) "char" as unsigned by default. Android does this for its ARM architectures and while we did fix this by enforcing -fsigned-char to its compiler, the issue may come up for other ports. Also it should be better to clarify in the engine when a variable is not actually storing string characters.

I've spotted and changed the most "safe" cases I could find for this. As far as I can tell, out of these, only the Outline field and fontoutline[] array could potentially be assigned valid negative values which would cause bugs for ports treating chars as unsigned.

In order to specify that they should be treated as signed

Some ports may treat an (unspecified) "char" as unsigned by default. Android does this for its ARM architectures and while we did fix this by enforcing -fsigned-char to its compiler, the issue may come up for other ports. Also it should be better to clarify in the engine when a variable is not actually storing string characters.

I've spotted and changed the most "safe" cases I could find for this. As far as I can tell, out of these, only the Outline field and fontoutline array could potentially be assigned valid negative values which would cause bugs for ports treating chars as unsigned.
@dreammaster dreammaster merged commit d474ee3 into scummvm:master Sep 10, 2021
8 checks passed
@dreammaster
Copy link
Member

@dreammaster dreammaster commented Sep 10, 2021

Thanks for doing review. It's appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants