Fix SGX implementations of read/write_vectored. #59009
Conversation
|
r? @rkruppe (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/libstd/sys/sgx/net.rs
Outdated
| pub fn read_vectored(&self, bufs: &mut [IoVecMut<'_>]) -> io::Result<usize> { | ||
| match bufs.iter_mut().find(|b| !b.is_empty()) { | ||
| Some(buf) => self.read(buf), | ||
| None => Ok(0), |
There was a problem hiding this comment.
Is this correct? On at least Linux & Windows, read(&mut []) blocks until the fd is ready for reading.
There was a problem hiding this comment.
Sure, we can change the default behavior to fall back to an empty buffer read.
|
I think I may be missing some context, but there's two changes here right? One is the fix to the sgx module which looks for the first nonempty buffer instead of always using the first. the second change is to always defer to (nothing against it of course, just wanted to explore rationale) |
|
This PR used to fix only SGX. But now it just generalizes this logic which is needed in multiple places and uses it everywhere it's needed. |
|
Yep, that's correct. The first part just fixes SGX and unifies the default implementations, and the second makes a behavior change to the defaults. We want to specify that read_vectored/write_vectored should behave as a read/write would be to a buffer that's the concatenation of the vectored input buffers. Previously though, if you didn't pass any nonempty buffers we'd just return Ok(0) directly rather than actually making a read/write call with an empty buffer. @jethrogb pointed out that empty reads/writes actually do have interesting behavior so I changed the defaults up to ensure we always call read/write from the default read_vectored/write_vectored implementation. Hopefully that makes sense! I can split the changes up into 2 PRs if you'd like. |
|
Ok thanks for confirming. I don't really understand where reading/writing an empty buffer would be significant, but so long as it's intentional sounds good to me @bors: r+ |
|
📌 Commit ab8e1d2 has been approved by |
bors
added
S-waiting-on-bors
…chton Fix SGX implementations of read/write_vectored.
…chton Fix SGX implementations of read/write_vectored.
…chton Fix SGX implementations of read/write_vectored.
Rollup of 37 pull requests Successful merges: - #58854 (appveyor: Use VS2017 for all our images) - #58855 (std: Spin for a global malloc lock on wasm32) - #58873 (Fix "Auto-hide item methods documentation" setting) - #58901 (Change `std::fs::copy` to use `copyfile` on MacOS and iOS) - #58933 (Move alloc::prelude::* to alloc::prelude::v1, make alloc a subset of std) - #58938 (core: ensure VaList passes improper_ctypes lint) - #58941 (MIPS: add r6 support) - #58949 (SGX target: Expose thread id function in os module) - #58959 (Add release notes for PR #56243) - #58976 (Default to integrated `rust-lld` linker for UEFI targets) - #59009 (Fix SGX implementations of read/write_vectored.) - #59025 (Fix generic argument lookup for Self) - #59036 (Fix ICE in MIR pretty printing) - #59037 (Avoid some common false positives in intra doc link checking) - #59072 (we can now skip should_panic tests with the libtest harness) - #59079 (add suggestions to invalid macro item error) - #59082 (A few improvements to comments in user-facing crates) - #59102 (Consistent naming for duration_float methods and additional f32 methods) - #59118 (rustc: fix ICE when trait alias has bare Self) - #59139 (Unregress using scalar unions in constants.) - #59146 (Suggest return lifetime when there's only one named lifetime) - #59147 (Make std time tests more robust for platform differences) - #59152 (Stabilize Range*::contains.) - #59156 ([wg-async-await] Add regression test for #55809.) - #59158 (Revert "Don't generate minification variable if minification disabled") - #59169 (Add `-Z allow_features=...` flag) - #59173 (bootstrap: Default to a sensible llvm-suffix.) - #59175 (Don't run test launching `echo` since that doesn't exist on Windows) - #59180 (Use try blocks in rustc_codegen_ssa) - #59185 (No old chestnuts in iter::repeat docs) - #59201 (Remove restriction on isize/usize in repr(simd)) - #59204 (Output diagnostic information for rustdoc) - #59206 (Improved test output) - #59208 (Reduce a Code Repetition Related to Bit Operation) - #59212 (Add x86_64 musl host to the manifest) - #59221 (Option and Result: Add references to documentation of as_ref and as_mut) - #59231 (Stabilize Option::copied)
SGX target: fix std unit tests This fixes some tests and some code in the SGX sys implementation to make the `std` unit test suite pass. rust-lang#59009 must be merged first.
SGX target: fix std unit tests This fixes some tests and some code in the SGX sys implementation to make the `std` unit test suite pass. #59009 must be merged first.
No description provided.