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

Add test cases for issues #10544 & #10545 #10546

Merged
merged 4 commits into from
Jan 19, 2022
Merged

Add test cases for issues #10544 & #10545 #10546

merged 4 commits into from
Jan 19, 2022

Conversation

zagortenej
Copy link
Contributor

Added unit test cases as per @Simn's comment on #10545

@Simn
Copy link
Member

Simn commented Jan 15, 2022

I have fixed the eval case, and pushed a fix to hxcpp for the negative hexes: HaxeFoundation/hxcpp@cb38fcc

Somehow I ignored the fact that this has to be fixed in the libraries directly. We'll not fix this for neko then because I don't think we plan to have another neko release anyway. I've changed the failing tests to not run on neko accordingly.

Something also fails on HL, will have to look into that as well.

@Simn
Copy link
Member

Simn commented Jan 17, 2022

The Lua failure is something else we have to check.

And for some reason, HL does not fail on Windows. No idea what's the deal with that.

@RealyUniqueName
Copy link
Member

HL doesn't fail on my Linux too.
We could blindly add handling of + to HL's parseInt, but that doesn't sound like a perfect solution.

@Simn
Copy link
Member

Simn commented Jan 19, 2022

@Aurel300 Do you happen to have an idea why Std.parseInt("+123") would fail on some systems but not on others?

@Aurel300
Copy link
Member

@Aurel300 Do you happen to have an idea why Std.parseInt("+123") would fail on some systems but not on others?

It failed locally too so I tracked down the issue. On Windows, Hashlink uses wcstol directly: this define shadows _utoi on Windows. On Linux and Mac, that define does not exist, so a custom _utoi is provided here. It copies decimal chars and - into a (non-wide) string buffer and passes that to strtol. The oversight here is that + is not considered a valid character. Also note that hexadecimal parsing is implemented manually higher up the callstack—unsure why @ncannasse, since both wcstol and strtol should be able to handle that (docs).

The fix should be this:

diff --git a/src/std/ucs2.c b/src/std/ucs2.c
@@ -109,7 +109,7 @@ int utoi( const uchar *str, uchar **end ) {
        while( is_space_char(*str) ) str++;
        while( i < 16 ) {
                int c = *str++;
-               if( (c < '0' || c > '9') && c != '-' )
+               if( (c < '0' || c > '9') && c != '-' && c != '+' )
                        break;
                buf[i++] = (char)c;
        }

Simn added a commit to HaxeFoundation/hashlink that referenced this pull request Jan 19, 2022
@Simn
Copy link
Member

Simn commented Jan 19, 2022

Thanks for checking, that did the trick!

@Simn Simn merged commit 13ee075 into HaxeFoundation:development Jan 19, 2022
@zagortenej zagortenej deleted the issue_10545 branch January 19, 2022 14:54
crazyjul pushed a commit to playdigious/hashlink that referenced this pull request Jan 18, 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.

4 participants