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 definitions on NetBSD #3927

Merged
merged 1 commit into from
Sep 25, 2024
Merged

Add missing definitions on NetBSD #3927

merged 1 commit into from
Sep 25, 2024

Conversation

0323pin
Copy link
Contributor

@0323pin 0323pin commented Sep 15, 2024

This PR adds support for:
CLOCK_PROCESS_CPUTIME_ID
CLOCK_THREAD_CPUTIME_ID
sysctlnametomib

It replaces the following closed PRs:
#3926
#3923

Sorry for the back and forward actions.

@rustbot
Copy link
Collaborator

rustbot commented Sep 15, 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

@0323pin
Copy link
Contributor Author

0323pin commented Sep 15, 2024

@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 Sep 15, 2024
@0323pin
Copy link
Contributor Author

0323pin commented Sep 18, 2024

r? @tgross35

It looks like @JohnTitor is busy would you mind reviewing?

@rustbot rustbot assigned tgross35 and unassigned JohnTitor Sep 18, 2024
@tgross35
Copy link
Contributor

I keep an eye on all the PRs even if they aren't assigned to me, just a bit of a slow turn time on this repo. It looks like other BSDs (Apple and NetBSD) have these already, could it instead just be moved to bsd/mod.rs?

Also, got a link to the relevant headers and/or docs?

@rustbot author (just comment @rustbot ready if you either make or decline the change)

@0323pin
Copy link
Contributor Author

0323pin commented Sep 25, 2024

I keep an eye on all the PRs even if they aren't assigned to me

Thanks for reviewing the PR.

just a bit of a slow turn time on this repo.

Yeah, sorry I didn't meen to be pushy.

It looks like other BSDs (Apple and NetBSD) have these already, could it instead just be moved to bsd/mod.rs?

Hmm ... I think you are mixing these a bit.
The constants are defined for OpenBSD but, missing for NetBSD. Hence, this PR. As for sysctlnametomib function, this exists on NetBSD but, not on OpenBSD. So, adding these to bsd/mod.rs would break OpenBSD.

Also, got a link to the relevant headers and/or docs?

Sure,
CLOCK_...
sysctlnametomib

~> uname -rsv
NetBSD 10.99.12 NetBSD 10.99.12 (GENERIC) #0: Wed Sep 18 01:34:08 UTC 2024  [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC
~> grep -r "sysctlnametomib" /usr/include/sys/
/usr/include/sys/sysctl.h:int	sysctlnametomib(const char *, int *, size_t *);
~> grep -r "CLOCK_PROCESS_CPUTIME" /usr/include/sys/
/usr/include/sys/time.h:#define CLOCK_PROCESS_CPUTIME_ID	0x40000000
~> grep -r "CLOCK_THREAD_CPUTIME" /usr/include/sys/
/usr/include/sys/time.h:#define CLOCK_THREAD_CPUTIME_ID		0x20000000

@0323pin
Copy link
Contributor Author

0323pin commented Sep 25, 2024

@rustbot ready

@tgross35
Copy link
Contributor

Thanks for confirming everything, and for the links to check against. Maybe sysctlnametomib should match the NetBSD names of (sname, name, namelenp) rather than (name, mibp, sizep) used by FreeBSD and Apple?

Either way lgtm.

@0323pin
Copy link
Contributor Author

0323pin commented Sep 25, 2024

@tgross35

Maybe sysctlnametomib should match the NetBSD names of (sname, name, namelenp) rather than (name, mibp, sizep) used by FreeBSD and Apple?

Duh ... you're probably right. Let me check this tonight. If I push a change, you want the commits squashed, right?

@tgross35
Copy link
Contributor

Duh ... you're probably right. Let me check this tonight. If I push a change, you want the commits squashed, right?

Yes please, GH's squash+merge doesn't work great with the merge queue.

@rustbot author

@0323pin
Copy link
Contributor Author

0323pin commented Sep 25, 2024

@tgross35 Sorry, it seems like my git skills aren't that great 😞

Anything against if, I close the PR, redo it all and re-submit?

@tgross35
Copy link
Contributor

Lol, git can be unfriendly :) usually you just do git rebase -i main which will open an editor where you should see your two commits. Then, for everything except the first line (oldest commit), replace pick with either f/fixup (= combine commits with the above but discard the message, which is what you want here) or s/squash (same thing but append the message). Save, and then git push --force-with-lease.

(of course a new PR is fine worst case, but squashing is usually easier if you can get it working)

@0323pin
Copy link
Contributor Author

0323pin commented Sep 25, 2024

Not far from it :)
I think I've just missed git push --force-with-lease.

This PR adds support for:
  CLOCK_PROCESS_CPUTIME_ID
  CLOCK_THREAD_CPUTIME_ID
  sysctlnametomib

It replaces the following closed PRs:
#3926
#3923

Sorry for the back and forward actions.
@0323pin
Copy link
Contributor Author

0323pin commented Sep 25, 2024

@tgross35 That was it. Using --force-with-lease did the trick, thank you!

@rustbot ready

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.

Looks great, thanks!

@tgross35 tgross35 added this pull request to the merge queue Sep 25, 2024
Merged via the queue into rust-lang:main with commit 8ff67c1 Sep 25, 2024
41 checks passed
@0323pin
Copy link
Contributor Author

0323pin commented Sep 25, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants