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

adding new illumos ptsname_r call. #3867

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

devnexen
Copy link
Contributor

@devnexen devnexen commented Aug 24, 2024

while at it, fixing PTHREAD_MUTEX_DEFAULT which differs from solaris.

man page

@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2024

r? @tgross35

rustbot has assigned @tgross35.
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

@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2024

Some changes occurred in solarish module

cc @jclulow, @pfmooney

@devnexen
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 24, 2024
@@ -68,6 +68,8 @@ pub const B4000000: ::speed_t = 31;
// sys/systeminfo.h
pub const SI_ADDRESS_WIDTH: ::c_int = 520;

pub const PTHREAD_MUTEX_DEFAULT: ::c_int = 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

This define cannot be changed in libc so soon. It was only changed in illumos in February 2024. Any running systems which predate the change (which there are many) would potentially malfunction if newly-compiled rust binaries use this newer value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so we are ok if unit tests do not necessarily pass then ?

@tgross35
Copy link
Contributor

tgross35 commented Sep 1, 2024

The above change was reverted as suggested, @pfmooney does this now look reasonable to you?

Also @devnexen do you have links to relevant headers / manpages?

@devnexen
Copy link
Contributor Author

devnexen commented Sep 1, 2024

description updated

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thanks, I haven't been very successful finding illumos headers but that looks like a good source.

This lgtm but I'll give pfmooney a couple more days to double check.

@tgross35
Copy link
Contributor

tgross35 commented Oct 1, 2024

@pfmooney any chance you could double check this with the changes?

@tgross35
Copy link
Contributor

tgross35 commented Oct 15, 2024

This looks straightforward enough to me so I'll just merge it. I won't backport it for a few days at least, @pfmooney feel free to leave an ack/nack at any time.

@tgross35 tgross35 added this pull request to the merge queue Oct 15, 2024
Merged via the queue into rust-lang:main with commit 113efbf Oct 15, 2024
39 checks passed
@bors bors mentioned this pull request Oct 15, 2024
@tgross35 tgross35 mentioned this pull request Oct 16, 2024
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Oct 16, 2024
(backport <rust-lang#3867>)
(cherry picked from commit 2c6ad42)
@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 Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review 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