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.

Sign up for GitHub

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

Jump to bottom

Support confstr on Linux #3612

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Support confstr on Linux #3612

merged 1 commit into from
Nov 12, 2024
+95 −10

Conversation

magicant
Copy link
Contributor

@magicant magicant commented Mar 5, 2024 •
edited
Loading

Description

This pull request adds support for the confstr function on Linux and also defines the constants used as a parameter for the function.

I moved the function declaration from src/unix/bsd/apple/mod.rs to src/unix/mod.rs (rather than adding to src/unix/linux_like/linux/mod.rs) since it is part of POSIX so it should be available on any unix-like systems.

The constants for non-linux targets are not the scope of this pull request.

Sources

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 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

@magicant magicant marked this pull request as ready for review March 6, 2024 14:31
@magicant magicant marked this pull request as draft May 1, 2024 11:58
@magicant magicant marked this pull request as ready for review May 1, 2024 13:56
@magicant magicant mentioned this pull request Jul 12, 2024
Closed
@jgarzik
Copy link

jgarzik commented Jul 13, 2024

Pull request looks good to me. More comprehensive than #3771.

Maybe a main-branch-first approach is ideal?

@tgross35
Copy link
Contributor

tgross35 commented Aug 16, 2024 •
edited
Loading

@magicant could you rebase this and target main?

This should remove confstr from src/unix/linux_like/linux/gnu/mod.rs, as was added in #3771.

@rustbot author

tgross35
tgross35 reviewed Aug 16, 2024
View reviewed changes
src/unix/linux_like/linux/mod.rs
@@ -1634,6 +1634,34 @@ pub const _SC_XOPEN_STREAMS: ::c_int = 246;
pub const _SC_THREAD_ROBUST_PRIO_INHERIT: ::c_int = 247;
pub const _SC_THREAD_ROBUST_PRIO_PROTECT: ::c_int = 248;

pub const _CS_PATH: ::c_int = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least some of these should be added to other platforms, e.g. OpenBSD https://man.openbsd.org/confstr.3

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree re additional for other platforms.

A challenge: The constant is stable and POSIX-defined, but the constant's value is not guaranteed stable across POSIX platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope someone else will contribute the constant values for non-linux targets.

@magicant magicant changed the base branch from libc-0.2 to main October 2, 2024 14:52
@bors
Copy link
Contributor

bors commented Oct 2, 2024

☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts.

@magicant
Copy link
Contributor Author

magicant commented Oct 2, 2024

Rebased.

I also re-checked the header files and changed some constant definitions that I missed before.

@rustbot ready

@39555 39555 mentioned this pull request Oct 20, 2024
Draft
@bors
Copy link
Contributor

bors commented Nov 7, 2024

☔ The latest upstream changes (presumably #4018) made this pull request unmergeable. Please resolve the merge conflicts.

tgross35
tgross35 reviewed Nov 7, 2024
View reviewed changes
src/unix/linux_like/linux/mod.rs Outdated
Comment on lines 2013 to 2033
pub const _CS_GNU_LIBC_VERSION: ::c_int = 2;
pub const _CS_GNU_LIBPTHREAD_VERSION: ::c_int = 3;
Copy link
Contributor

@tgross35 tgross35 Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these _GNU_ values should go in gnu or musl only, they don't seem to exist in ulibc or newlib

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Those values are specific to glibc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Musl seems to have them https://github.com/kraj/musl/blob/ffb23aef7b5339b8c3234f4c6a93c488dc873919/include/unistd.h#L439-L440 which is probably why we aren't getting test failures. But I don't think we have any need to expose it on anything but gnu.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

649154d...51512d1

Moved back the _CS_GNU_LIB... definitions.

Thanks for reviewing!

@tgross35
Copy link
Contributor

tgross35 commented Nov 7, 2024

Sorry for the conflict, one last thing to double check above and then LGTM

@magicant
Copy link
Contributor Author

magicant commented Nov 9, 2024

51f9da4...649154d

Rebased and resolved the conflicts

@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Nov 12, 2024
@tgross35 tgross35 added this pull request to the merge queue Nov 12, 2024
Merged via the queue into rust-lang:main with commit d9b2927 Nov 12, 2024
42 checks passed
@magicant magicant deleted the confstr-linux branch November 12, 2024 12:15
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 13, 2024
@magicant @tgross35
Support confstr on Linux
5109d52 
(backport <rust-lang#3612>)
(cherry picked from commit 51512d1)
@tgross35 tgross35 mentioned this pull request Nov 13, 2024
Merged
@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 Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgarzik jgarzik jgarzik left review comments

@tgross35 tgross35 tgross35 approved these changes

Assignees

@JohnTitor JohnTitor

Labels
S-waiting-on-review stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@magicant @rustbot @jgarzik @tgross35 @bors @JohnTitor