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 missing error numbers for HermitOS #3858

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Conversation

stlankes
Copy link
Contributor

@stlankes stlankes commented Aug 19, 2024

HermitOS is a unikernel and its interface to the kernel is provided by https://crates.io/crates/hermit-abi. In the meantime parts of the interface is stabilized and we want to integrated it into libc. Unstable version will be still provided by hermit-abi.

Error numbers are missing in the main branch.

@rustbot
Copy link
Collaborator

rustbot commented Aug 19, 2024

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@stlankes
Copy link
Contributor Author

@rustbot label +stable-nominated

@rustbot rustbot added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Aug 19, 2024
@stlankes
Copy link
Contributor Author

See also #3766

@tgross35
Copy link
Contributor

tgross35 commented Aug 20, 2024

The numbers all looks correct to me. However, the linked source at https://github.com/hermit-os/hermit-rs/blob/5a4d7efe1ab5727cf4d5299943aa8deaa098a603/hermit-abi/src/errno.rs looks like it always uses i32, not c_int that is used in this PR. Probably better to just use i32 here as well?

Also, could you start a file libc-test/semver/hermit.txt and add these? This won't be checked in CI but just run the tests locally and make sure they pass.

@rustbot author

@stlankes
Copy link
Contributor Author

c_int is defined as i32 (https://github.com/rust-lang/libc/blob/main/src/hermit.rs#L15). Hm, it irritates a little. We thought that this easier to understand because all other OS are using c_int. We can change it, I am open for it.

Also, could you start a file libc-test/semver/hermit.txt and add these? This won't be checked in CI but just run the tests locally and make sure they pass.

I will do it

@tgross35
Copy link
Contributor

c_int is defined as i32 (https://github.com/rust-lang/libc/blob/main/src/hermit.rs#L15). Hm, it irritates a little. We thought that this easier to understand because all other OS are using c_int. We can change it, I am open for it.

Regardless of how the type aliases are defined, prefer being consistent with the relevant headers / source files over being consistent with other platforms. Doesn't really matter here since c_int is almost always i32, but it makes it easier to verify that we don't quietly introduce bugs.

Looks like this is already done elsewhere, e.g. SO_REUSEADDR is a c_int on most platforms but an i32 on Hermit.

@stlankes
Copy link
Contributor Author

stlankes commented Aug 20, 2024

Ok, I thought about it. I will change it to i32.

@tgross35
Copy link
Contributor

Thanks! Can you add libc-test/semver/hermit.txt?

@stlankes
Copy link
Contributor Author

I just added libc-test/semver/hermit.txt.

@tgross35
Copy link
Contributor

tgross35 commented Aug 20, 2024

I only meant to include the API added here, the rest could come later. But even better that you did it all now I suppose :)

Thanks!

@tgross35 tgross35 added this pull request to the merge queue Aug 21, 2024
Merged via the queue into rust-lang:main with commit 1178c25 Aug 21, 2024
40 checks passed
@stlankes stlankes deleted the hermit branch August 21, 2024 06:38
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 28, 2024
(backport <rust-lang#3858>)
(cherry picked from commit d448050)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 28, 2024
(backport <rust-lang#3858>)
(cherry picked from commit 982e041)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 28, 2024
(backport <rust-lang#3858>)
(cherry picked from commit d448050)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 28, 2024
(backport <rust-lang#3858>)
(cherry picked from commit 982e041)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 28, 2024
(backport <rust-lang#3858>)
(cherry picked from commit d448050)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Aug 28, 2024
(backport <rust-lang#3858>)
(cherry picked from commit 982e041)
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants