Conversation
|
Avoiding unsafe code is nice, but I don't think we should be avoiding going through libc. In particular on Android, where we use this crate in several parts of the platform, there are a number of places where libc has important platform logic (e.g. for tracking file descriptors) that shouldn't be skipped, and so we would like to avoid using If there are some more safe wrappers in |
If this still worries you, the |
|
Unfortunately as of bytecodealliance/rustix#1528 rustix uses |
|
|
|
Actually, my understanding of the above PR is that it isn't making Android use the raw system call backend. Even if |
8ee606f to
ebc06ab
Compare
vsock-rs currently uses a mix of nix and libc to handle the underlying system calls. However, in addition to adding a few extra dependencies, this means that there is a significant amount of unneeded unsafe code in the project. By porting to "rustix", we gain the following advantages: - Most of the code in the project can be rewritten completely safely. - We avoid going through libc. In other projects this has been benchmarked to lead to a 2.5% performance increase. - On Linux, we reduce the number of dependencies the project pulls in. On other platforms, it stays the same. Blocked on bytecodealliance/rustix#1558 Signed-off-by: John Nunley <dev@notgull.net>
ebc06ab to
1e90f22
Compare
Rustix has gone back-and-forth on it's policy on Android. Android platform developers don't want a dependency on linux-raw-sys. This is documented here: bytecodealliance#1095 bytecodealliance#1478 rust-vsock/vsock-rs#60 Android app developers seem to want to use linux-raw-sys though: bytecodealliance#1528 The Android platform developers don't REALLY care about the cargo build though. CI/CD testing on the particular build options Android likes is useful. This MR adds another non-cargo build system (Meson) to the CI/CD, that mirrors the options Android likes. Meson is renowned for being a easy-to-maintain build system. By adding it here, we ensure the continue use of Rustix in core Android platform code.
Rustix has gone back-and-forth on it's policy on Android. Android platform developers don't want a dependency on linux-raw-sys. This is documented here: bytecodealliance#1095 bytecodealliance#1478 rust-vsock/vsock-rs#60 Android app developers seem to want to use linux-raw-sys though: bytecodealliance#1528 The Android platform developers don't REALLY care about the cargo build though. CI/CD testing on the particular build options Android likes is useful. This MR adds another non-cargo build system (Meson) to the CI/CD, that mirrors the options Android likes. Meson is renowned for being a easy-to-maintain build system. By adding it here, we ensure the continue use of Rustix in core Android platform code.
Rustix has gone back-and-forth on it's policy on Android. Android platform developers don't want a dependency on linux-raw-sys. This is documented here: bytecodealliance#1095 bytecodealliance#1478 rust-vsock/vsock-rs#60 Android app developers seem to want to use linux-raw-sys though: bytecodealliance#1528 The Android platform developers don't REALLY care about the cargo build though. CI/CD testing on the particular build options Android likes is useful. This MR adds another non-cargo build system (Meson) to the CI/CD, that mirrors the options Android likes. Meson is renowned for being a easy-to-maintain build system. By adding it here, we ensure the continue use of Rustix in core Android platform code.
Rustix has gone back-and-forth on it's policy on Android. Android platform developers don't want a dependency on linux-raw-sys. This is documented here: bytecodealliance#1095 bytecodealliance#1478 rust-vsock/vsock-rs#60 Android app developers seem to want to use linux-raw-sys though: bytecodealliance#1528 The Android platform developers don't REALLY care about the cargo build though. CI/CD testing on the particular build options Android likes is useful. This MR adds another non-cargo build system (Meson) to the CI/CD, that mirrors the options Android likes. Meson is renowned for being a easy-to-maintain build system and arguably the favorite among Linux-distro maintainers. Right now, meson currently does not use Cargo.toml as a source of truth for dependency, since a dependency's build.rs file can basically do anything, and there's no way for an external build system to determine a build.rs file is doing [1]. As such, the standard way to include dependencies is are meson subprojects, for each dependency. There are various proposals to improve the situation [2], but those would require a bleeding edge meson not in CI/CD bots yet. That said, there's an easy way to test from the root of the directory: sudo apt install meson ninja-build meson setup rustix-build ninja -C rustix-build Meson closely mirrors what Android does. By adding it here, we ensure the continue use of Rustix in core Android platform code. [1] mesonbuild/meson#2173 [2] mesonbuild/meson#15223
| #[cfg(target_os = "macos")] | ||
| let flags = SockFlag::empty(); | ||
| Ok(socket(AddressFamily::Vsock, ty, flags, None)?) | ||
| let flags = net::SocketFlags::empty(); |
There was a problem hiding this comment.
Behavior change? We set CLOEXEC outside of macos
vsock-rs currently uses a mix of nix and libc to handle
the underlying system calls. However, in addition to adding
a few extra dependencies, this means that there is a significant
amount of unneeded unsafe code in the project.
By porting to "rustix", we gain the following advantages:
safely.
benchmarked to lead to a 2.5% performance increase.
pulls in. On other platforms, it stays the same.
This is a breaking change, since this crate exposes primitives from nix; specifically "VsockAddr". I've added a new "SocketAddr" type that exposes no types from any subcrates.
Blocked on bytecodealliance/rustix#1558