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

Add missing sysctl net types for mac #4022

Merged
merged 2 commits into from
Nov 16, 2024
Merged

Add missing sysctl net types for mac #4022

merged 2 commits into from
Nov 16, 2024
+73 −1

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Nov 10, 2024 •
edited by tgross35
Loading

Thanks to GuillaumeGomez/sysinfo#1378, I discovered that mac's API was actually 32 bits for network information, even if it's not obvious. To go around this limitation, we need to use a different kind of sysctl value and struct. Adding them here.

Sources

@rustbot
Copy link
Collaborator

rustbot commented Nov 10, 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

@GuillaumeGomez
Copy link
Member Author

Ah I see, the net/if_mib.h is not included for mac's libc-test. Fixing that.

@GuillaumeGomez GuillaumeGomez force-pushed the net-sysctl-types branch 3 times, most recently from 9424e40 to 75d537c Compare November 10, 2024 14:20
@GuillaumeGomez
Copy link
Member Author

And implemented traits on the new types as well.

@GuillaumeGomez
Copy link
Member Author

CI passed. \o/

tgross35
tgross35 reviewed Nov 14, 2024
View reviewed changes
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.

One cleanup suggestion but otherwise lgtm. Also added source links to the PR description.

@rustbot label +stable-nominated

src/unix/bsd/apple/mod.rs Outdated
@@ -1580,6 +1580,50 @@ s_no_extra_traits! {
pub ifr_name: [::c_char; ::IFNAMSIZ],
pub ifr_ifru: __c_anonymous_ifr_ifru6,
}

pub struct ifmibdata {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could these just go in s! rather than s_no_extra_traits! so the trait implementations don't need to be handwritten?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I was wondering why I needed to implement these traits myself. Completely forgot about the macro... ^^'

@rustbot rustbot added stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Nov 14, 2024
@GuillaumeGomez
Copy link
Member Author

Not sure to understand why linux is failing considering I didn't change linux's code... Weird rustc-llvm error.

tgross35
tgross35 approved these changes Nov 14, 2024
View reviewed changes
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.

Yeah the CI failure is unrelated rust-lang/rust#133035, I'll merge after the next nightly drops.

@tgross35 tgross35 enabled auto-merge November 16, 2024 05:02
@tgross35 tgross35 added this pull request to the merge queue Nov 16, 2024
@tgross35 tgross35 added stable-blocked and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Nov 16, 2024
Merged via the queue into rust-lang:main with commit d01b57b Nov 16, 2024
43 checks passed
@GuillaumeGomez GuillaumeGomez deleted the net-sysctl-types branch November 16, 2024 09:59
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 16, 2024
@GuillaumeGomez @tgross35
Add missing sysctl net types for mac
61da9bc 
(backport <rust-lang#4022>)
(cherry picked from commit 57a1be4)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 16, 2024
@GuillaumeGomez @tgross35
Add net/if_mib.h header for mac test build
23f263e 
(backport <rust-lang#4022>)
(cherry picked from commit 1bcda05)
@tgross35 tgross35 mentioned this pull request Nov 16, 2024
Merged
@tgross35 tgross35 added stable-nominated This PR should be considered for cherry-pick to libc's stable release branch stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-blocked stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Nov 16, 2024
@ydirson ydirson mentioned this pull request Nov 21, 2024
Open
@tgross35
Copy link
Contributor

Hey @GuillaumeGomez where did the definition for if_family_id come from? I only found it at https://fergofrog.com/code/codebrowser/xnu/bsd/net/if_mib.h.html but it doesn't seem to exist in xnu https://github.com/apple-oss-distributions/xnu/blob/8d741a5de7ff4191bf97d57b9f54c2f6d4a15585/bsd/net/if_mib.h. These tests are failing on my machine, I'm not sure how they are passing in CI.

@GuillaumeGomez
Copy link
Member Author

It's the second header: https://fergofrog.com/code/codebrowser/xnu/bsd/net/if_mib.h.html#if_family_id

@tgross35
Copy link
Contributor

I know that site shows it, but where does that site's source come from? It's not in the official xnu repo

@GuillaumeGomez
Copy link
Member Author

Absolutely no clue. Seems to be a common header for BSD-based systems from what I can understand. It's horribly difficult to find apple's headers. When I tested locally on my (2015) macbook, there was no issue so I assumed it was the right definition.

@tgross35
Copy link
Contributor

Mine is a bit newer, I have the header but I don't have that specific struct (matches xnu). Maybe it existed at some point but was removed? For reference xcodebuild -showsdks shows

macOS SDKs:
        macOS 15.0                      -sdk macosx15.0
        macOS 15.0                      -sdk macosx15.0

It's not a problem to have if they're useful, just wondering if there is a test config that should be set so mine don't fail :)

@GuillaumeGomez
Copy link
Member Author

Ah. If it was removed then we have a big issue. OS versions don't mix well at all with APIs.

@tgross35
Copy link
Contributor

tgross35 commented Nov 23, 2024 •
edited
Loading

It looks like CI has the macOS 14.5 SDKs, so I'm going to guess that it isn't available in the newest APIs. Did you need if_family_id specifically or would you mind if I just removed that type? (it doesn't seem to exist in any function signatures)

@GuillaumeGomez
Copy link
Member Author

I don't need it so go ahead. :)

tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 24, 2024
@tgross35
apple: remove if_family_id
e87acba 
This API appears to not be available in more recent MacOS SDKs, and
there aren't any functions that use it. Since this hasn't yet made it
into a release, remove it.

Link: rust-lang#4022
@tgross35 tgross35 mentioned this pull request Nov 24, 2024
Merged
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
apple: remove if_family_id
631f82c 
This API appears to not be available in more recent MacOS SDKs, and
there aren't any functions that use it. Since this hasn't yet made it
into a release, remove it.

Link: rust-lang#4022

(backport <rust-lang#4137>)
(cherry picked from commit e87acba)
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
apple: remove if_family_id
a129371 
This API appears to not be available in more recent MacOS SDKs, and
there aren't any functions that use it. Since this hasn't yet made it
into a release, remove it.

Link: rust-lang#4022

(backport <rust-lang#4137>)
(cherry picked from commit e87acba)
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
apple: remove if_family_id
63ebc8e 
This API appears to not be available in more recent MacOS SDKs, and
there aren't any functions that use it. Since this hasn't yet made it
into a release, remove it.

Link: rust-lang#4022

(backport <rust-lang#4137>)
(cherry picked from commit e87acba)
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 25, 2024
@tgross35
apple: remove if_family_id
fe4def5 
This API appears to not be available in more recent MacOS SDKs, and
there aren't any functions that use it. Since this hasn't yet made it
into a release, remove it.

Link: rust-lang#4022

(backport <rust-lang#4137>)
(cherry picked from commit e87acba)
@andrewdavidmackenzie andrewdavidmackenzie mentioned this pull request Nov 26, 2024
Closed
@shadow3aaa shadow3aaa mentioned this pull request Dec 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgross35 tgross35 tgross35 approved these changes

Assignees

@JohnTitor JohnTitor

Labels
O-macos O-unix S-waiting-on-author 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.
4 participants
@GuillaumeGomez @rustbot @tgross35 @JohnTitor