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

Signed qualifier required for working compile on some systems. #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HPCguy
Copy link

@HPCguy HPCguy commented Apr 15, 2022

No description provided.

@chqrlie
Copy link

chqrlie commented Dec 7, 2024

This patch takes a complicated approach that might not be supported in a simple compiler where the signed keyword can be considered useless. A simpler fix should be:

-    while (std_fns[0] != -1)
+    while (std_fns[0] != '\xff')

'\xx' is negative on platforms where char has 8 bits and is signed by default.

@HPCguy
Copy link
Author

HPCguy commented Dec 8, 2024

It has been a long time since I submitted this.

The only x86 system I have is an HP chromebook [AMD A4-9120C RADEON R4, 5 COMPUTE CORES 2C+3G (2 threads, 1.60GHz)] so I am guessing it was the Chromebook Linux Development Environment GCC compiler I had 'apt install' ed at the time. The only other compiler I have run on that system is https://github.com/EarlGray/c4 , but I highly doubt that was the one I was using. Hence, probably GCC.

@HPCguy
Copy link
Author

HPCguy commented Dec 8, 2024

PS I like your solution better, although I can't test because I have lost the test directory I was using when I had to make the submitted change on this pull request to make the code run. Feel free to close this pull request, but I recommend a fix, because it definitely broke, and took me a minute to figure out why it was crashing in the first place, only to find that my submitted patch made it work.

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