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

implement ifreq for linux #2960

Merged
merged 1 commit into from Oct 16, 2022
Merged

implement ifreq for linux #2960

merged 1 commit into from Oct 16, 2022

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Oct 13, 2022

defines the ifreq struct, for use with ioctl. I used the definition from the manpages

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@folkertdev
Copy link
Contributor Author

I'm not sure what the CI error means. It seems to complain about type I did not touch?!

src/unix/linux_like/linux/mod.rs Outdated Show resolved Hide resolved
src/unix/linux_like/linux/mod.rs Outdated Show resolved Hide resolved
@JohnTitor
Copy link
Member

Okay I checked the source and realized that it's anonymous: https://github.com/torvalds/linux/blob/5bfc75d92efd494db37f5c4c173d3639d4772966/include/uapi/linux/if.h#L234-L275

Luckily, we have a prior art on FreeBSD: #2556
I think following a similar way would make CI green, but I'd suggest using them on your local once, to confirm the behavior.

@folkertdev
Copy link
Contributor Author

I can't seem to run the tests locally, at least not in the way that the README describes

> cargo test
   Compiling libc-test v0.2.135 (/home/folkertdev/rust/libc/libc-test)
The following warnings were emitted during compilation:

warning: /home/folkertdev/rust/libc/target/debug/build/libc-test-dbaae0e5290e28ae/out/main.c:129:10: fatal error: linux/openat2.h: No such file or directory
warning:   129 | #include <linux/openat2.h>
warning:       |          ^~~~~~~~~~~~~~~~~
warning: compilation terminated.

should this work? I ran these installs based on the dockerfile: gcc libc6-dev ca-certificates linux-headers-generic.

In any case, the implementation now more closely mimics the freebsd one. I added a Debug impl, but not Eq or Hash because the linux version of the ifru union contains a pointer, so equality/hashing would not have the behavior we typically expect in rust.

@JohnTitor
Copy link
Member

JohnTitor commented Oct 14, 2022

I guess your glibc/OS version is old, openat2.h was introduced in Linux 5.6. For example, the libc-test currently requires Ubuntu 22.04 and its glibc version.

@folkertdev
Copy link
Contributor Author

ok, well not much I can do about that in the short run. I may not have fully understood your suggestion: is there something in the freebsd code that I did not copy (correctly) that would fix the current CI issue?

@JohnTitor
Copy link
Member

is there something in the freebsd code that I did not copy (correctly) that would fix the current CI issue?

Looking at the source, ifru_map is also anonymous, it seems. And about in6_ifreq, it's declared on include/uapi/linux/ipv6.h but the test doesn't include it, you have to add it to test_linux func of https://github.com/rust-lang/libc/blob/master/libc-test/build.rs (I'm not sure if it requires a special handling like in6.h though). I don't think these get rid of all the errors, but let's do them and check what errors remain.

@folkertdev
Copy link
Contributor Author

allright, got CI to pass. Thanks @JohnTitor for your help!

@JohnTitor
Copy link
Member

LGTM, could you make sure these are usable for your use case? (If you already did it, great!) Also, it'd be great if you could squash commits into one as the current commits would be noisy when git-blaming.

@folkertdev
Copy link
Contributor Author

commits are squashed, and I verified that this works for our use case

@JohnTitor
Copy link
Member

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Oct 16, 2022

📌 Commit 7266718 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 16, 2022

⌛ Testing commit 7266718 with merge 937aeaa...

@bors
Copy link
Contributor

bors commented Oct 16, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing 937aeaa to master...

@bors bors merged commit 937aeaa into rust-lang:master Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants