Skip to content

Add CI #10

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

Merged
merged 3 commits into from
Oct 18, 2022
Merged

Add CI #10

merged 3 commits into from
Oct 18, 2022

Conversation

eldruin
Copy link
Member

@eldruin eldruin commented Aug 31, 2022

I noticed one failing test for snprintf.
As the original author, is that something you could look into @thejpster?

Fixes #9

@jonathanpallant
Copy link
Collaborator

I think the test matrix is wrong. You can only pass the lp32 feature on platforms where Long and Pointers are 32-bit, which is not true on x86_64-unknown-linux-gnu. The difference in length is cause because -100i32 is passed to sprintf("%ld") and -100i32 isn't a long, and the wrong number of bytes is read giving a bad (much longer) string output.

@eldruin
Copy link
Member Author

eldruin commented Sep 1, 2022

I see. Do you know any architecture available in cross that I can use to run the tests for LP32? What about the no-features case?

@jonathanpallant
Copy link
Collaborator

i686-something would be LP32 targets. You can target that on an x86-64 machine without using cross, I believe.

@jonathanpallant
Copy link
Collaborator

(I mean, you can /target/ anything but of course the point of the tests is to execute them, and an x86-64 machine will execute i686 code just fine).

@jonathanpallant
Copy link
Collaborator

I can't remember what happens with no features, and I'm afraid I'm not in a position to check right now. I seem to recall the code only checks LP64 and not(LP64) and only when looking at %ld for snprintf, but I could be wrong.

@thejpster
Copy link
Member

Can you rebase this once the other PR is in? Maybe our issues will go away.

@eldruin eldruin force-pushed the master branch 3 times, most recently from f55e064 to 58dcc3f Compare October 18, 2022 06:28
@eldruin
Copy link
Member Author

eldruin commented Oct 18, 2022

They are indeed! This can now be reviewed and merged.

@thejpster thejpster merged commit a1c832b into rust-embedded-community:master Oct 18, 2022
@thejpster
Copy link
Member

Thanks!

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.

Add CI
3 participants