From abff464d28f410da75f73d434e0a6c529b1949b5 Mon Sep 17 00:00:00 2001 From: Jeff Vander Stoep Date: Tue, 6 Feb 2024 10:07:03 +0100 Subject: Upgrade vmm-sys-util to 0.12.1 This project was upgraded with external_updater. Usage: tools/external_updater/updater.sh update external/rust/crates/vmm-sys-util For more info, check https://cs.android.com/android/platform/superproject/+/main:tools/external_updater/README.md Test: TreeHugger Change-Id: I14cbee7a488882399996a693e4cd6780d6c34ea8 --- .cargo_vcs_info.json | 2 +- Android.bp | 2 +- CHANGELOG.md | 23 +++++ CODEOWNERS | 2 +- Cargo.toml | 21 +++- Cargo.toml.orig | 7 +- METADATA | 21 ++-- coverage_config_x86_64.json | 2 +- src/fam.rs | 240 ++++++++++++++++++++++++++++++-------------- src/lib.rs | 3 +- src/linux/aio.rs | 2 +- src/linux/epoll.rs | 35 ++----- src/linux/fallocate.rs | 1 + src/linux/poll.rs | 44 +++++++- src/linux/seek_hole.rs | 26 ++--- src/linux/sock_ctrl_msg.rs | 16 +-- src/linux/timerfd.rs | 1 + src/linux/write_zeroes.rs | 24 ++--- src/rand.rs | 26 ++++- src/syscall.rs | 18 +++- src/tempfile.rs | 28 +++--- src/unix/tempdir.rs | 1 + 22 files changed, 373 insertions(+), 172 deletions(-) diff --git a/.cargo_vcs_info.json b/.cargo_vcs_info.json index e83a746..137eab1 100644 --- a/.cargo_vcs_info.json +++ b/.cargo_vcs_info.json @@ -1,6 +1,6 @@ { "git": { - "sha1": "0e10ca98b55797a64319d746d94379f7cdf81d02" + "sha1": "5eae197d43cbe41f601bfb449fbc0579e5967847" }, "path_in_vcs": "" } \ No newline at end of file diff --git a/Android.bp b/Android.bp index 75c7bbd..b25068f 100644 --- a/Android.bp +++ b/Android.bp @@ -5,7 +5,7 @@ rust_library_host { name: "libvmm_sys_util", crate_name: "vmm_sys_util", cargo_env_compat: true, - cargo_pkg_version: "0.11.1", + cargo_pkg_version: "0.12.1", srcs: ["src/lib.rs"], edition: "2021", rustlibs: [ diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a08682..dd5145d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,27 @@ # Changelog + +## v0.12.1 + +### Changed +- [[#215](https://github.com/rust-vmm/vmm-sys-util/pull/215)]: Make + `as_mut_fam_struct()` public and unsafe to let users modify fields of the + header other than the length. + +## v0.12.0 + +### Changed +- Added all features to the generated docs.rs documentation. +- Fixed a bug in `serde` implementation of `FamStructWrapper` which allowed out of + bounds memory access from Rust-safe code. See the related GHSA here: + https://github.com/rust-vmm/vmm-sys-util/security/advisories/GHSA-875g-mfp6-g7f9 + for more information. + +## v0.11.2 + +### Changed +- [[#201](https://github.com/rust-vmm/vmm-sys-util/pull/201)] Updated `SyscallReturnCode` + to accept any signed integer type. + ## v0.11.1 ### Changed diff --git a/CODEOWNERS b/CODEOWNERS index b0440e9..c650fc4 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @liujing2 @sameo @andreeaflorescu @jiangliu +* @liujing2 @sameo @andreeaflorescu @jiangliu @roypat @JonathanWoollett-Light diff --git a/Cargo.toml b/Cargo.toml index f1da15c..2719169 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,14 +12,21 @@ [package] edition = "2021" name = "vmm-sys-util" -version = "0.11.1" +version = "0.12.1" authors = ["Intel Virtualization Team "] description = "A system utility set" readme = "README.md" keywords = ["utils"] license = "BSD-3-Clause" repository = "https://github.com/rust-vmm/vmm-sys-util" -resolver = "2" + +[package.metadata.docs.rs] +all-features = true +rustdoc-args = [ + "--cfg", + "docsrs", +] + [dependencies.libc] version = "0.2.39" @@ -30,10 +37,18 @@ optional = true [dependencies.serde_derive] version = "1.0.27" optional = true + +[dev-dependencies.bincode] +version = "1.3.3" + [dev-dependencies.serde_json] version = "1.0.9" [features] -with-serde = ["serde", "serde_derive"] +with-serde = [ + "serde", + "serde_derive", +] + [target."cfg(any(target_os = \"linux\", target_os = \"android\"))".dependencies.bitflags] version = "1.0" diff --git a/Cargo.toml.orig b/Cargo.toml.orig index 668a2ac..70e92b5 100644 --- a/Cargo.toml.orig +++ b/Cargo.toml.orig @@ -1,6 +1,6 @@ [package] name = "vmm-sys-util" -version = "0.11.1" +version = "0.12.1" authors = ["Intel Virtualization Team "] description = "A system utility set" repository = "https://github.com/rust-vmm/vmm-sys-util" @@ -9,6 +9,10 @@ keywords = ["utils"] edition = "2021" license = "BSD-3-Clause" +[package.metadata.docs.rs] +all-features = true +rustdoc-args = ["--cfg", "docsrs"] + [features] with-serde = ["serde", "serde_derive"] @@ -22,3 +26,4 @@ bitflags = "1.0" [dev-dependencies] serde_json = "1.0.9" +bincode = "1.3.3" diff --git a/METADATA b/METADATA index 1c8b946..4eb3799 100644 --- a/METADATA +++ b/METADATA @@ -1,19 +1,24 @@ +# This project was upgraded with external_updater. +# Usage: tools/external_updater/updater.sh update external/rust/crates/vmm-sys-util +# For more info, check https://cs.android.com/android/platform/superproject/+/main:tools/external_updater/README.md + name: "vmm-sys-util" description: "A system utility set" third_party { + license_type: NOTICE + last_upgrade_date { + year: 2024 + month: 2 + day: 6 + } identifier { type: "crates.io" - value: "https://crates.io/crates/vmm-sys-util" + value: "https://static.crates.io/crates/vmm-sys-util/vmm-sys-util-0.12.1.crate" + version: "0.11.1" } identifier { type: "Archive" value: "https://static.crates.io/crates/vmm-sys-util/vmm-sys-util-0.11.1.crate" - } - version: "0.11.1" - license_type: NOTICE - last_upgrade_date { - year: 2023 - month: 8 - day: 23 + version: "0.12.1" } } diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 4277fc0..2c55cb3 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 87.1, + "coverage_score": 84.39, "exclude_path": "", "crate_features": "" } diff --git a/src/fam.rs b/src/fam.rs index 0d62b0f..13ba4c2 100644 --- a/src/fam.rs +++ b/src/fam.rs @@ -50,6 +50,8 @@ impl fmt::Display for Error { /// * the implementer should be a POD /// * the implementor should contain a flexible array member of elements of type `Entry` /// * `Entry` should be a POD +/// * the implementor should ensures that the FAM length as returned by [`FamStruct::len()`] +/// always describes correctly the length of the flexible array member. /// /// Violating these may cause problems. /// @@ -99,7 +101,7 @@ impl fmt::Display for Error { /// self.len as usize /// } /// -/// fn set_len(&mut self, len: usize) { +/// unsafe fn set_len(&mut self, len: usize) { /// self.len = len as u32 /// } /// @@ -135,7 +137,12 @@ pub unsafe trait FamStruct { /// /// These type of structures contain a member that holds the FAM length. /// This method will set the value of that member. - fn set_len(&mut self, len: usize); + /// + /// # Safety + /// + /// The caller needs to ensure that `len` here reflects the correct number of entries of the + /// flexible array part of the struct. + unsafe fn set_len(&mut self, len: usize); /// Get max allowed FAM length /// @@ -169,9 +176,14 @@ impl FamStructWrapper { /// /// Get the capacity required by mem_allocator in order to hold /// the provided number of [`FamStruct::Entry`](trait.FamStruct.html#associatedtype.Entry). - fn mem_allocator_len(fam_len: usize) -> usize { - let wrapper_size_in_bytes = size_of::() + fam_len * size_of::(); - (wrapper_size_in_bytes + size_of::() - 1) / size_of::() + /// Returns `None` if the required length would overflow usize. + fn mem_allocator_len(fam_len: usize) -> Option { + let wrapper_size_in_bytes = + size_of::().checked_add(fam_len.checked_mul(size_of::())?)?; + + wrapper_size_in_bytes + .checked_add(size_of::().checked_sub(1)?)? + .checked_div(size_of::()) } /// Convert `mem_allocator` len to FAM len. @@ -206,7 +218,8 @@ impl FamStructWrapper { return Err(Error::SizeLimitExceeded); } let required_mem_allocator_capacity = - FamStructWrapper::::mem_allocator_len(num_elements); + FamStructWrapper::::mem_allocator_len(num_elements) + .ok_or(Error::SizeLimitExceeded)?; let mut mem_allocator = Vec::with_capacity(required_mem_allocator_capacity); mem_allocator.push(T::default()); @@ -214,7 +227,11 @@ impl FamStructWrapper { // SAFETY: Safe as long T follows the requirements of being POD. mem_allocator.push(unsafe { mem::zeroed() }) } - mem_allocator[0].set_len(num_elements); + // SAFETY: The flexible array part of the struct has `num_elements` capacity. We just + // initialized this in `mem_allocator`. + unsafe { + mem_allocator[0].set_len(num_elements); + } Ok(FamStructWrapper { mem_allocator }) } @@ -234,7 +251,8 @@ impl FamStructWrapper { let mut adapter = FamStructWrapper::::new(entries.len())?; { - let wrapper_entries = adapter.as_mut_fam_struct().as_mut_slice(); + // SAFETY: We are not modifying the length of the FamStruct + let wrapper_entries = unsafe { adapter.as_mut_fam_struct().as_mut_slice() }; wrapper_entries.copy_from_slice(entries); } @@ -271,7 +289,12 @@ impl FamStructWrapper { } /// Get a mut reference to the actual [`FamStruct`](trait.FamStruct.html) instance. - pub fn as_mut_fam_struct(&mut self) -> &mut T { + /// + /// # Safety + /// + /// Callers must not use the reference returned to modify the `len` filed of the underlying + /// `FamStruct`. See also the top-level documentation of [`FamStruct`]. + pub unsafe fn as_mut_fam_struct(&mut self) -> &mut T { &mut self.mem_allocator[0] } @@ -294,7 +317,8 @@ impl FamStructWrapper { /// Modifying the container referenced by this pointer may cause its buffer /// to be reallocated, which would also make any pointers to it invalid. pub fn as_mut_fam_struct_ptr(&mut self) -> *mut T { - self.as_mut_fam_struct() + // SAFETY: We do not change the length of the underlying FamStruct. + unsafe { self.as_mut_fam_struct() } } /// Get the elements slice. @@ -304,7 +328,8 @@ impl FamStructWrapper { /// Get the mutable elements slice. pub fn as_mut_slice(&mut self) -> &mut [T::Entry] { - self.as_mut_fam_struct().as_mut_slice() + // SAFETY: We do not change the length of the underlying FamStruct. + unsafe { self.as_mut_fam_struct() }.as_mut_slice() } /// Get the number of elements of type `FamStruct::Entry` currently in the vec. @@ -326,17 +351,20 @@ impl FamStructWrapper { /// /// If the capacity is already reserved, this method doesn't do anything. /// If not this will trigger a reallocation of the underlying buffer. - fn reserve(&mut self, additional: usize) { + fn reserve(&mut self, additional: usize) -> Result<(), Error> { let desired_capacity = self.len() + additional; if desired_capacity <= self.capacity() { - return; + return Ok(()); } let current_mem_allocator_len = self.mem_allocator.len(); - let required_mem_allocator_len = FamStructWrapper::::mem_allocator_len(desired_capacity); + let required_mem_allocator_len = FamStructWrapper::::mem_allocator_len(desired_capacity) + .ok_or(Error::SizeLimitExceeded)?; let additional_mem_allocator_len = required_mem_allocator_len - current_mem_allocator_len; self.mem_allocator.reserve(additional_mem_allocator_len); + + Ok(()) } /// Update the length of the FamStructWrapper. @@ -352,7 +380,10 @@ impl FamStructWrapper { /// /// When len is greater than the max possible len it returns Error::SizeLimitExceeded. fn set_len(&mut self, len: usize) -> Result<(), Error> { - let additional_elements: isize = len as isize - self.len() as isize; + let additional_elements = isize::try_from(len) + .and_then(|len| isize::try_from(self.len()).map(|self_len| len - self_len)) + .map_err(|_| Error::SizeLimitExceeded)?; + // If len == self.len there's nothing to do. if additional_elements == 0 { return Ok(()); @@ -365,11 +396,12 @@ impl FamStructWrapper { return Err(Error::SizeLimitExceeded); } // Reserve additional capacity. - self.reserve(additional_elements as usize); + self.reserve(additional_elements as usize)?; } let current_mem_allocator_len = self.mem_allocator.len(); - let required_mem_allocator_len = FamStructWrapper::::mem_allocator_len(len); + let required_mem_allocator_len = + FamStructWrapper::::mem_allocator_len(len).ok_or(Error::SizeLimitExceeded)?; // Update the len of the `mem_allocator`. // SAFETY: This is safe since enough capacity has been reserved. unsafe { @@ -382,7 +414,11 @@ impl FamStructWrapper { self.mem_allocator[i] = unsafe { mem::zeroed() } } // Update the len of the underlying `FamStruct`. - self.as_mut_fam_struct().set_len(len); + // SAFETY: We just adjusted the memory for the underlying `mem_allocator` to hold `len` + // entries. + unsafe { + self.as_mut_fam_struct().set_len(len); + } // If the len needs to be decreased, deallocate unnecessary memory if additional_elements < 0 { @@ -445,9 +481,9 @@ impl PartialEq for FamStructWrapper { impl Clone for FamStructWrapper { fn clone(&self) -> Self { // The number of entries (self.as_slice().len()) can't be > T::max_len() since `self` is a - // valid `FamStructWrapper`. + // valid `FamStructWrapper`. This makes the .unwrap() safe. let required_mem_allocator_capacity = - FamStructWrapper::::mem_allocator_len(self.as_slice().len()); + FamStructWrapper::::mem_allocator_len(self.as_slice().len()).unwrap(); let mut mem_allocator = Vec::with_capacity(required_mem_allocator_capacity); @@ -469,7 +505,7 @@ impl Clone for FamStructWrapper { let mut adapter = FamStructWrapper { mem_allocator }; { - let wrapper_entries = adapter.as_mut_fam_struct().as_mut_slice(); + let wrapper_entries = adapter.as_mut_slice(); wrapper_entries.copy_from_slice(self.as_slice()); } adapter @@ -527,13 +563,23 @@ where { use serde::de::Error; - let header = seq + let header: X = seq .next_element()? .ok_or_else(|| de::Error::invalid_length(0, &self))?; let entries: Vec = seq .next_element()? .ok_or_else(|| de::Error::invalid_length(1, &self))?; + if header.len() != entries.len() { + let msg = format!( + "Mismatch between length of FAM specified in FamStruct header ({}) \ + and actual size of FAM ({})", + header.len(), + entries.len() + ); + return Err(V::Error::custom(msg)); + } + let mut result: Self::Value = FamStructWrapper::from_entries(entries.as_slice()) .map_err(|e| V::Error::custom(format!("{:?}", e)))?; result.mem_allocator[0] = header; @@ -557,7 +603,7 @@ macro_rules! generate_fam_struct_impl { self.$field_name as usize } - fn set_len(&mut self, len: usize) { + unsafe fn set_len(&mut self, len: usize) { self.$field_name = len as $field_type; } @@ -581,6 +627,7 @@ macro_rules! generate_fam_struct_impl { #[cfg(test)] mod tests { #![allow(clippy::undocumented_unsafe_blocks)] + #[cfg(feature = "with-serde")] use serde_derive::{Deserialize, Serialize}; @@ -589,7 +636,7 @@ mod tests { const MAX_LEN: usize = 100; #[repr(C)] - #[derive(Default, PartialEq, Eq)] + #[derive(Default, Debug, PartialEq, Eq)] pub struct __IncompleteArrayField(::std::marker::PhantomData, [T; 0]); impl __IncompleteArrayField { #[inline] @@ -678,12 +725,30 @@ mod tests { let fam_len = pair.0; let mem_allocator_len = pair.1; assert_eq!( - mem_allocator_len, + Some(mem_allocator_len), MockFamStructWrapper::mem_allocator_len(fam_len) ); } } + #[repr(C)] + #[derive(Default, PartialEq)] + struct MockFamStructU8 { + pub len: u32, + pub padding: u32, + pub entries: __IncompleteArrayField, + } + generate_fam_struct_impl!(MockFamStructU8, u8, entries, u32, len, 100); + type MockFamStructWrapperU8 = FamStructWrapper; + #[test] + fn test_invalid_type_conversion() { + let mut adapter = MockFamStructWrapperU8::new(10).unwrap(); + assert!(matches!( + adapter.set_len(0xffff_ffff_ffff_ff00), + Err(Error::SizeLimitExceeded) + )); + } + #[test] fn test_wrapper_len() { for pair in MEM_ALLOCATOR_LEN_TO_FAM_LEN { @@ -785,7 +850,7 @@ mod tests { let num_elements = pair.0; let required_mem_allocator_len = pair.1; - adapter.reserve(num_elements); + adapter.reserve(num_elements).unwrap(); assert!(adapter.mem_allocator.capacity() >= required_mem_allocator_len); assert_eq!(0, adapter.len()); @@ -794,7 +859,7 @@ mod tests { // test that when the capacity is already reserved, the method doesn't do anything let current_capacity = adapter.capacity(); - adapter.reserve(current_capacity - 1); + adapter.reserve(current_capacity - 1).unwrap(); assert_eq!(current_capacity, adapter.capacity()); } @@ -831,7 +896,8 @@ mod tests { assert_eq!(adapter.as_slice()[i], i as u32); assert_eq!(adapter.len(), i + 1); assert!( - adapter.mem_allocator.capacity() >= MockFamStructWrapper::mem_allocator_len(i + 1) + adapter.mem_allocator.capacity() + >= MockFamStructWrapper::mem_allocator_len(i + 1).unwrap() ); } @@ -858,7 +924,7 @@ mod tests { assert_eq!(adapter.len(), num_retained_entries); assert!( adapter.mem_allocator.capacity() - >= MockFamStructWrapper::mem_allocator_len(num_retained_entries) + >= MockFamStructWrapper::mem_allocator_len(num_retained_entries).unwrap() ); } @@ -910,7 +976,7 @@ mod tests { assert_eq!(payload[0], 0xA5); assert_eq!(payload[1], 0x1e); } - assert_eq!(wrapper.as_mut_fam_struct().padding, 5); + assert_eq!(unsafe { wrapper.as_mut_fam_struct() }.padding, 5); let data = wrapper.into_raw(); assert_eq!(data[0].len, 2); assert_eq!(data[0].padding, 5); @@ -996,53 +1062,81 @@ mod tests { type FooFamStructWrapper = FamStructWrapper; let mut wrapper = FooFamStructWrapper::new(0).unwrap(); - wrapper.as_mut_fam_struct().index = 1; - wrapper.as_mut_fam_struct().flags = 2; - wrapper.as_mut_fam_struct().length = 3; - wrapper.push(3).unwrap(); - wrapper.push(14).unwrap(); - assert_eq!(wrapper.as_slice().len(), 3 + 2); - assert_eq!(wrapper.as_slice()[3], 3); - assert_eq!(wrapper.as_slice()[3 + 1], 14); - - let mut wrapper2 = wrapper.clone(); - assert_eq!( - wrapper.as_mut_fam_struct().index, - wrapper2.as_mut_fam_struct().index - ); - assert_eq!( - wrapper.as_mut_fam_struct().length, - wrapper2.as_mut_fam_struct().length - ); - assert_eq!( - wrapper.as_mut_fam_struct().flags, - wrapper2.as_mut_fam_struct().flags - ); - assert_eq!(wrapper.as_slice(), wrapper2.as_slice()); - assert_eq!( - wrapper2.as_slice().len(), - wrapper2.as_mut_fam_struct().length as usize - ); - assert!(wrapper == wrapper2); + // SAFETY: We do play with length here, but that's just for testing purposes :) + unsafe { + wrapper.as_mut_fam_struct().index = 1; + wrapper.as_mut_fam_struct().flags = 2; + wrapper.as_mut_fam_struct().length = 3; + wrapper.push(3).unwrap(); + wrapper.push(14).unwrap(); + assert_eq!(wrapper.as_slice().len(), 3 + 2); + assert_eq!(wrapper.as_slice()[3], 3); + assert_eq!(wrapper.as_slice()[3 + 1], 14); + + let mut wrapper2 = wrapper.clone(); + assert_eq!( + wrapper.as_mut_fam_struct().index, + wrapper2.as_mut_fam_struct().index + ); + assert_eq!( + wrapper.as_mut_fam_struct().length, + wrapper2.as_mut_fam_struct().length + ); + assert_eq!( + wrapper.as_mut_fam_struct().flags, + wrapper2.as_mut_fam_struct().flags + ); + assert_eq!(wrapper.as_slice(), wrapper2.as_slice()); + assert_eq!( + wrapper2.as_slice().len(), + wrapper2.as_mut_fam_struct().length as usize + ); + assert!(wrapper == wrapper2); - wrapper.as_mut_fam_struct().index = 3; - assert!(wrapper != wrapper2); + wrapper.as_mut_fam_struct().index = 3; + assert!(wrapper != wrapper2); - wrapper.as_mut_fam_struct().length = 7; - assert!(wrapper != wrapper2); + wrapper.as_mut_fam_struct().length = 7; + assert!(wrapper != wrapper2); - wrapper.push(1).unwrap(); - assert_eq!(wrapper.as_mut_fam_struct().length, 8); - assert!(wrapper != wrapper2); + wrapper.push(1).unwrap(); + assert_eq!(wrapper.as_mut_fam_struct().length, 8); + assert!(wrapper != wrapper2); - let mut wrapper2 = wrapper.clone(); - assert!(wrapper == wrapper2); + let mut wrapper2 = wrapper.clone(); + assert!(wrapper == wrapper2); - // Dropping the original variable should not affect its clone. - drop(wrapper); - assert_eq!(wrapper2.as_mut_fam_struct().index, 3); - assert_eq!(wrapper2.as_mut_fam_struct().length, 8); - assert_eq!(wrapper2.as_mut_fam_struct().flags, 2); - assert_eq!(wrapper2.as_slice(), [0, 0, 0, 3, 14, 0, 0, 1]); + // Dropping the original variable should not affect its clone. + drop(wrapper); + assert_eq!(wrapper2.as_mut_fam_struct().index, 3); + assert_eq!(wrapper2.as_mut_fam_struct().length, 8); + assert_eq!(wrapper2.as_mut_fam_struct().flags, 2); + assert_eq!(wrapper2.as_slice(), [0, 0, 0, 3, 14, 0, 0, 1]); + } + } + + #[cfg(feature = "with-serde")] + #[test] + fn test_bad_deserialize() { + #[repr(C)] + #[derive(Default, Debug, PartialEq, Serialize, Deserialize)] + struct Foo { + pub len: u32, + pub padding: u32, + pub entries: __IncompleteArrayField, + } + + generate_fam_struct_impl!(Foo, u32, entries, u32, len, 100); + + let state = FamStructWrapper::::new(0).unwrap(); + let mut bytes = bincode::serialize(&state).unwrap(); + + // The `len` field of the header is the first to be serialized. + // Writing at position 0 of the serialized data should change its value. + bytes[0] = 255; + + assert!( + matches!(bincode::deserialize::>(&bytes).map_err(|boxed| *boxed), Err(bincode::ErrorKind::Custom(s)) if s == *"Mismatch between length of FAM specified in FamStruct header (255) and actual size of FAM (0)") + ); } } diff --git a/src/lib.rs b/src/lib.rs index 1929816..0467825 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,7 +4,8 @@ //! Collection of modules that provides helpers and utilities used by multiple //! [rust-vmm](https://github.com/rust-vmm/community) components. -#![deny(missing_docs)] +#![deny(missing_docs, missing_debug_implementations)] +#![cfg_attr(docsrs, feature(doc_auto_cfg))] #[cfg(any(target_os = "linux", target_os = "android"))] mod linux; diff --git a/src/linux/aio.rs b/src/linux/aio.rs index 1e14ea0..98bc2cf 100644 --- a/src/linux/aio.rs +++ b/src/linux/aio.rs @@ -223,7 +223,7 @@ impl IoContext { ) -> Result { let to = match timeout { Some(val) => val as *mut libc::timespec, - None => null_mut() as *mut libc::timespec, + None => null_mut(), }; // SAFETY: It's safe because parameters are valid and we have checked the result. diff --git a/src/linux/epoll.rs b/src/linux/epoll.rs index b8e9b7b..c68d1f9 100644 --- a/src/linux/epoll.rs +++ b/src/linux/epoll.rs @@ -19,6 +19,7 @@ use libc::{ use crate::syscall::SyscallReturnCode; /// Wrapper over `EPOLL_CTL_*` operations that can be performed on a file descriptor. +#[derive(Debug)] #[repr(i32)] pub enum ControlOperation { /// Add a file descriptor to the interest list. @@ -385,20 +386,12 @@ mod tests { // For EPOLL_CTL_ADD behavior we will try to add some fds with different event masks into // the interest list of epoll instance. assert!(epoll - .ctl( - ControlOperation::Add, - event_fd_1.as_raw_fd() as i32, - event_1 - ) + .ctl(ControlOperation::Add, event_fd_1.as_raw_fd(), event_1) .is_ok()); // We can't add twice the same fd to epoll interest list. assert!(epoll - .ctl( - ControlOperation::Add, - event_fd_1.as_raw_fd() as i32, - event_1 - ) + .ctl(ControlOperation::Add, event_fd_1.as_raw_fd(), event_1) .is_err()); let event_fd_2 = EventFd::new(libc::EFD_NONBLOCK).unwrap(); @@ -406,7 +399,7 @@ mod tests { assert!(epoll .ctl( ControlOperation::Add, - event_fd_2.as_raw_fd() as i32, + event_fd_2.as_raw_fd(), // For this fd, we want an Event instance that has `data` field set to other // value than the value of the fd and `events` without EPOLLIN type set. EpollEvent::new(EventSet::OUT, 10) @@ -419,11 +412,7 @@ mod tests { let event_fd_3 = EventFd::new(libc::EFD_NONBLOCK).unwrap(); let event_3 = EpollEvent::new(EventSet::OUT | EventSet::IN, event_fd_3.as_raw_fd() as u64); assert!(epoll - .ctl( - ControlOperation::Add, - event_fd_3.as_raw_fd() as i32, - event_3 - ) + .ctl(ControlOperation::Add, event_fd_3.as_raw_fd(), event_3) .is_ok()); // Let's check `epoll_wait()` behavior for our epoll instance. @@ -457,11 +446,7 @@ mod tests { // that we want to monitor this time on event_fd_1. event_1 = EpollEvent::new(EventSet::OUT, 20); assert!(epoll - .ctl( - ControlOperation::Modify, - event_fd_1.as_raw_fd() as i32, - event_1 - ) + .ctl(ControlOperation::Modify, event_fd_1.as_raw_fd(), event_1) .is_ok()); let event_fd_4 = EventFd::new(libc::EFD_NONBLOCK).unwrap(); @@ -469,7 +454,7 @@ mod tests { assert!(epoll .ctl( ControlOperation::Modify, - event_fd_4.as_raw_fd() as i32, + event_fd_4.as_raw_fd(), EpollEvent::default() ) .is_err()); @@ -485,7 +470,7 @@ mod tests { assert!(epoll .ctl( ControlOperation::Modify, - event_fd_1.as_raw_fd() as i32, + event_fd_1.as_raw_fd(), EpollEvent::default() ) .is_ok()); @@ -498,7 +483,7 @@ mod tests { assert!(epoll .ctl( ControlOperation::Delete, - event_fd_2.as_raw_fd() as i32, + event_fd_2.as_raw_fd(), EpollEvent::default() ) .is_ok()); @@ -514,7 +499,7 @@ mod tests { assert!(epoll .ctl( ControlOperation::Delete, - event_fd_4.as_raw_fd() as i32, + event_fd_4.as_raw_fd(), EpollEvent::default() ) .is_err()); diff --git a/src/linux/fallocate.rs b/src/linux/fallocate.rs index e3a7fed..904bff2 100644 --- a/src/linux/fallocate.rs +++ b/src/linux/fallocate.rs @@ -14,6 +14,7 @@ use crate::errno::{errno_result, Error, Result}; /// Operation to be performed on a given range when calling [`fallocate`] /// /// [`fallocate`]: fn.fallocate.html +#[derive(Debug)] pub enum FallocateMode { /// Deallocating file space. PunchHole, diff --git a/src/linux/poll.rs b/src/linux/poll.rs index 12809f0..54dd4b9 100644 --- a/src/linux/poll.rs +++ b/src/linux/poll.rs @@ -47,6 +47,12 @@ const POLL_CONTEXT_MAX_EVENTS: usize = 16; /// This should only be used with [`EpollContext`](struct.EpollContext.html). pub struct EpollEvents(RefCell<[epoll_event; POLL_CONTEXT_MAX_EVENTS]>); +impl std::fmt::Debug for EpollEvents { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "EpollEvents {{ ... }}") + } +} + impl EpollEvents { /// Creates a new EpollEvents. pub fn new() -> EpollEvents { @@ -90,7 +96,7 @@ impl PollToken for usize { impl PollToken for u64 { fn as_raw_token(&self) -> u64 { - *self as u64 + *self } fn from_raw_token(data: u64) -> Self { @@ -142,6 +148,15 @@ pub struct PollEvent<'a, T> { token: PhantomData, // Needed to satisfy usage of T } +impl std::fmt::Debug for PollEvent<'_, T> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PollEvent") + .field("event", &"?") + .field("token", &self.token) + .finish() + } +} + impl<'a, T: PollToken> PollEvent<'a, T> { /// Gets the token associated in /// [`PollContext::add`](struct.PollContext.html#method.add) with this event. @@ -189,6 +204,7 @@ impl<'a, T: PollToken> PollEvent<'a, T> { /// An iterator over a subset of events returned by /// [`PollContext::wait`](struct.PollContext.html#method.wait). +#[derive(Debug)] pub struct PollEventIter<'a, I, T> where I: Iterator, @@ -222,6 +238,16 @@ pub struct PollEvents<'a, T> { tokens: PhantomData<[T]>, // Needed to satisfy usage of T } +impl std::fmt::Debug for PollEvents<'_, T> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PollEventsOwned") + .field("count", &self.count) + .field("events", &"?") + .field("tokens", &self.tokens) + .finish() + } +} + impl<'a, T: PollToken> PollEvents<'a, T> { /// Creates owned structure from borrowed [`PollEvents`](struct.PollEvents.html). /// @@ -270,6 +296,16 @@ pub struct PollEventsOwned { tokens: PhantomData, // Needed to satisfy usage of T } +impl std::fmt::Debug for PollEventsOwned { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PollEventsOwned") + .field("count", &self.count) + .field("events", &"?") + .field("tokens", &self.tokens) + .finish() + } +} + impl PollEventsOwned { /// Creates borrowed structure from owned structure /// [`PollEventsOwned`](struct.PollEventsOwned.html). @@ -286,7 +322,7 @@ impl PollEventsOwned { } /// Watching events taken by [`PollContext`](struct.PollContext.html). -#[derive(Copy, Clone)] +#[derive(Debug, Copy, Clone)] pub struct WatchingEvents(u32); impl WatchingEvents { @@ -355,6 +391,7 @@ impl WatchingEvents { /// assert_eq!(event.token(), 1); /// } /// ``` +#[derive(Debug)] pub struct EpollContext { epoll_ctx: File, // Needed to satisfy usage of T @@ -699,6 +736,7 @@ impl IntoRawFd for EpollContext { /// let tokens: Vec = pollevents.iter_readable().map(|e| e.token()).collect(); /// assert_eq!(&tokens[..], &[2]); /// ``` +#[derive(Debug)] pub struct PollContext { epoll_ctx: EpollContext, @@ -834,6 +872,8 @@ impl PollContext { let mut buf = [0u8; 512]; let (res, len) = { let mut buf_cursor = Cursor::new(&mut buf[..]); + // Oops, clippy bug. See https://github.com/rust-lang/rust-clippy/issues/9810 + #[allow(clippy::write_literal)] ( writeln!( &mut buf_cursor, diff --git a/src/linux/seek_hole.rs b/src/linux/seek_hole.rs index 1392993..6c35455 100644 --- a/src/linux/seek_hole.rs +++ b/src/linux/seek_hole.rs @@ -84,7 +84,7 @@ mod tests { use std::path::PathBuf; fn seek_cur(file: &mut File) -> u64 { - file.seek(SeekFrom::Current(0)).unwrap() + file.stream_position().unwrap() } #[test] @@ -154,7 +154,7 @@ mod tests { assert_eq!(seek_cur(&mut file), 0xFFFF); // seek_hole at or after the end of the file should return None - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x10000).unwrap(), None); assert_eq!(seek_cur(&mut file), 0); assert_eq!(file.seek_hole(0x10001).unwrap(), None); @@ -172,18 +172,18 @@ mod tests { assert_eq!(seek_cur(&mut file), 0xFFFF); // seek_hole within data should return the next hole (EOF) - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x10000).unwrap(), Some(0x20000)); assert_eq!(seek_cur(&mut file), 0x20000); - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x10001).unwrap(), Some(0x20000)); assert_eq!(seek_cur(&mut file), 0x20000); - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x1FFFF).unwrap(), Some(0x20000)); assert_eq!(seek_cur(&mut file), 0x20000); // seek_hole at EOF after data should return None - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x20000).unwrap(), None); assert_eq!(seek_cur(&mut file), 0); @@ -193,21 +193,21 @@ mod tests { assert_eq!(seek_cur(&mut file), 0); assert_eq!(file.seek_hole(0xFFFF).unwrap(), Some(0xFFFF)); assert_eq!(seek_cur(&mut file), 0xFFFF); - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x10000).unwrap(), Some(0x20000)); assert_eq!(seek_cur(&mut file), 0x20000); - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x1FFFF).unwrap(), Some(0x20000)); assert_eq!(seek_cur(&mut file), 0x20000); - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x20000).unwrap(), Some(0x20000)); assert_eq!(seek_cur(&mut file), 0x20000); - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x20001).unwrap(), Some(0x20001)); assert_eq!(seek_cur(&mut file), 0x20001); // seek_hole at EOF after a hole should return None - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x30000).unwrap(), None); assert_eq!(seek_cur(&mut file), 0); @@ -218,10 +218,10 @@ mod tests { // seek_hole within [0x20000, 0x30000) should now find the hole at EOF assert_eq!(file.seek_hole(0x20000).unwrap(), Some(0x30000)); assert_eq!(seek_cur(&mut file), 0x30000); - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x20001).unwrap(), Some(0x30000)); assert_eq!(seek_cur(&mut file), 0x30000); - file.seek(SeekFrom::Start(0)).unwrap(); + file.rewind().unwrap(); assert_eq!(file.seek_hole(0x30000).unwrap(), None); assert_eq!(seek_cur(&mut file), 0); } diff --git a/src/linux/sock_ctrl_msg.rs b/src/linux/sock_ctrl_msg.rs index 0c19a11..9e69e2b 100644 --- a/src/linux/sock_ctrl_msg.rs +++ b/src/linux/sock_ctrl_msg.rs @@ -98,7 +98,10 @@ fn set_msg_controllen(msg: &mut msghdr, cmsg_capacity: usize) { // This function is like CMSG_NEXT, but safer because it reads only from references, although it // does some pointer arithmetic on cmsg_ptr. -#[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] +#[cfg_attr( + feature = "cargo-clippy", + allow(clippy::cast_ptr_alignment, clippy::unnecessary_cast) +)] fn get_next_cmsg(msghdr: &msghdr, cmsg: &cmsghdr, cmsg_ptr: *mut cmsghdr) -> *mut cmsghdr { let next_cmsg = (cmsg_ptr as *mut u8).wrapping_add(CMSG_ALIGN!(cmsg.cmsg_len)) as *mut cmsghdr; if next_cmsg @@ -151,7 +154,7 @@ impl CmsgBuffer { } fn raw_sendmsg(fd: RawFd, out_data: &[D], out_fds: &[RawFd]) -> Result { - let cmsg_capacity = CMSG_SPACE!(size_of::() * out_fds.len()); + let cmsg_capacity = CMSG_SPACE!(std::mem::size_of_val(out_fds)); let mut cmsg_buffer = CmsgBuffer::with_capacity(cmsg_capacity); let mut iovecs = Vec::with_capacity(out_data.len()); @@ -166,7 +169,7 @@ fn raw_sendmsg(fd: RawFd, out_data: &[D], out_fds: &[RawFd]) -> Re if !out_fds.is_empty() { let cmsg = cmsghdr { - cmsg_len: CMSG_LEN!(size_of::() * out_fds.len()), + cmsg_len: CMSG_LEN!(std::mem::size_of_val(out_fds)), cmsg_level: SOL_SOCKET, cmsg_type: SCM_RIGHTS, #[cfg(all(target_env = "musl", target_pointer_width = "64"))] @@ -175,7 +178,7 @@ fn raw_sendmsg(fd: RawFd, out_data: &[D], out_fds: &[RawFd]) -> Re // SAFETY: Check comments below for each call. unsafe { // Safe because cmsg_buffer was allocated to be large enough to contain cmsghdr. - write_unaligned(cmsg_buffer.as_mut_ptr() as *mut cmsghdr, cmsg); + write_unaligned(cmsg_buffer.as_mut_ptr(), cmsg); // Safe because the cmsg_buffer was allocated to be large enough to hold out_fds.len() // file descriptors. copy_nonoverlapping( @@ -200,12 +203,13 @@ fn raw_sendmsg(fd: RawFd, out_data: &[D], out_fds: &[RawFd]) -> Re } } +#[cfg_attr(feature = "cargo-clippy", allow(clippy::unnecessary_cast))] unsafe fn raw_recvmsg( fd: RawFd, iovecs: &mut [iovec], in_fds: &mut [RawFd], ) -> Result<(usize, usize)> { - let cmsg_capacity = CMSG_SPACE!(size_of::() * in_fds.len()); + let cmsg_capacity = CMSG_SPACE!(std::mem::size_of_val(in_fds)); let mut cmsg_buffer = CmsgBuffer::with_capacity(cmsg_capacity); let mut msg = new_msghdr(iovecs); @@ -241,7 +245,7 @@ unsafe fn raw_recvmsg( // read. let cmsg = (cmsg_ptr as *mut cmsghdr).read_unaligned(); if cmsg.cmsg_level == SOL_SOCKET && cmsg.cmsg_type == SCM_RIGHTS { - let fds_count = ((cmsg.cmsg_len - CMSG_LEN!(0)) as usize) / size_of::(); + let fds_count: usize = ((cmsg.cmsg_len - CMSG_LEN!(0)) as usize) / size_of::(); // The sender can transmit more data than we can buffer. If a message is too long to // fit in the supplied buffer, excess bytes may be discarded depending on the type of // socket the message is received from. diff --git a/src/linux/timerfd.rs b/src/linux/timerfd.rs index 80f0789..a67198b 100644 --- a/src/linux/timerfd.rs +++ b/src/linux/timerfd.rs @@ -19,6 +19,7 @@ use crate::errno::{errno_result, Result}; /// A safe wrapper around a Linux /// [`timerfd`](http://man7.org/linux/man-pages/man2/timerfd_create.2.html). +#[derive(Debug)] pub struct TimerFd(File); impl TimerFd { diff --git a/src/linux/write_zeroes.rs b/src/linux/write_zeroes.rs index e6084da..4c9e77b 100644 --- a/src/linux/write_zeroes.rs +++ b/src/linux/write_zeroes.rs @@ -28,7 +28,7 @@ pub trait PunchHole { impl PunchHole for File { fn punch_hole(&mut self, offset: u64, length: u64) -> Result<()> { - fallocate(self, FallocateMode::PunchHole, true, offset, length as u64) + fallocate(self, FallocateMode::PunchHole, true, offset, length) .map_err(|e| Error::from_raw_os_error(e.errno())) } } @@ -140,7 +140,7 @@ impl WriteZeroesAt for File { impl WriteZeroes for T { fn write_zeroes(&mut self, length: usize) -> Result { - let offset = self.seek(SeekFrom::Current(0))?; + let offset = self.stream_position()?; let num_written = self.write_zeroes_at(offset, length)?; // Advance the seek cursor as if we had done a real write(). self.seek(SeekFrom::Current(num_written as i64))?; @@ -171,7 +171,7 @@ mod tests { // Read back the data plus some overlap on each side. let mut readback = [0u8; 16384]; - f.seek(SeekFrom::Start(0)).unwrap(); + f.rewind().unwrap(); f.read_exact(&mut readback).unwrap(); // Bytes before the write should still be 0. for read in &readback[0..1234] { @@ -190,10 +190,10 @@ mod tests { f.seek(SeekFrom::Start(2345)).unwrap(); f.write_all_zeroes(4321).unwrap(); // Verify seek position after `write_all_zeroes()`. - assert_eq!(f.seek(SeekFrom::Current(0)).unwrap(), 2345 + 4321); + assert_eq!(f.stream_position().unwrap(), 2345 + 4321); // Read back the data and verify that it is now zero. - f.seek(SeekFrom::Start(0)).unwrap(); + f.rewind().unwrap(); f.read_exact(&mut readback).unwrap(); // Bytes before the write should still be 0. for read in &readback[0..1234] { @@ -228,19 +228,19 @@ mod tests { // Write buffer of non-zero bytes. The size of the buffer will be the new // size of the file. let orig_data = [NON_ZERO_VALUE; SIZE]; - f.seek(SeekFrom::Start(0)).unwrap(); + f.rewind().unwrap(); f.write_all(&orig_data).unwrap(); assert_eq!(f.metadata().unwrap().len(), SIZE as u64); // Overwrite some of the data with zeroes. - f.seek(SeekFrom::Start(0)).unwrap(); + f.rewind().unwrap(); f.write_all_zeroes(0x1_0001).unwrap(); // Verify seek position after `write_all_zeroes()`. - assert_eq!(f.seek(SeekFrom::Current(0)).unwrap(), 0x1_0001); + assert_eq!(f.stream_position().unwrap(), 0x1_0001); // Read back the data and verify that it is now zero. let mut readback = [0u8; SIZE]; - f.seek(SeekFrom::Start(0)).unwrap(); + f.rewind().unwrap(); f.read_exact(&mut readback).unwrap(); // Verify that `write_all_zeroes()` zeroed the intended region. for read in &readback[0..0x1_0001] { @@ -253,7 +253,7 @@ mod tests { // Now let's zero a certain region by using `write_all_zeroes_at()`. f.write_all_zeroes_at(0x1_8001, 0x200).unwrap(); - f.seek(SeekFrom::Start(0)).unwrap(); + f.rewind().unwrap(); f.read_exact(&mut readback).unwrap(); // Original data should still exist before the zeroed region. @@ -281,7 +281,7 @@ mod tests { // Write buffer of non-zero bytes. The size of the buffer will be the new // size of the file. let orig_data = [NON_ZERO_VALUE; SIZE]; - f.seek(SeekFrom::Start(0)).unwrap(); + f.rewind().unwrap(); f.write_all(&orig_data).unwrap(); assert_eq!(f.metadata().unwrap().len(), SIZE as u64); @@ -291,7 +291,7 @@ mod tests { // Read back the data. let mut readback = [0u8; SIZE]; - f.seek(SeekFrom::Start(0)).unwrap(); + f.rewind().unwrap(); f.read_exact(&mut readback).unwrap(); // Original data should still exist before the hole. for read in &readback[0..0x1_0001] { diff --git a/src/rand.rs b/src/rand.rs index 097341f..8a88dd3 100644 --- a/src/rand.rs +++ b/src/rand.rs @@ -19,7 +19,7 @@ pub fn timestamp_cycles() -> u64 { #[cfg(target_arch = "x86_64")] // SAFETY: Safe because there's nothing that can go wrong with this call. unsafe { - std::arch::x86_64::_rdtsc() as u64 + std::arch::x86_64::_rdtsc() } #[cfg(not(target_arch = "x86_64"))] @@ -124,13 +124,21 @@ mod tests { // The 33 will be discarded as it is not a valid letter // (upper or lower) or number. let s = xor_pseudo_rng_u8_alphanumerics(&|| i); - assert_eq!(vec![54, 55], s); + if cfg!(target_endian = "big") { + assert_eq!(vec![55, 54], s); + } else { + assert_eq!(vec![54, 55], s); + } } #[test] fn test_rand_alphanumerics_impl() { let s = rand_alphanumerics_impl(&|| 14134, 5); - assert_eq!("67676", s); + if cfg!(target_endian = "big") { + assert_eq!("76767", s); + } else { + assert_eq!("67676", s); + } } #[test] @@ -145,13 +153,21 @@ mod tests { // The 33 will be discarded as it is not a valid letter // (upper or lower) or number. let s = xor_pseudo_rng_u8_bytes(&|| i); - assert_eq!(vec![54, 33, 55, 0], s); + if cfg!(target_endian = "big") { + assert_eq!(vec![0, 55, 33, 54], s); + } else { + assert_eq!(vec![54, 33, 55, 0], s); + } } #[test] fn test_rand_bytes_impl() { let s = rand_bytes_impl(&|| 1234567, 4); - assert_eq!(vec![135, 214, 18, 0], s); + if cfg!(target_endian = "big") { + assert_eq!(vec![0, 18, 214, 135], s); + } else { + assert_eq!(vec![135, 214, 18, 0], s); + } } #[test] diff --git a/src/syscall.rs b/src/syscall.rs index 6fb4d64..a89209a 100644 --- a/src/syscall.rs +++ b/src/syscall.rs @@ -6,12 +6,13 @@ use std::os::raw::c_int; /// Wrapper to interpret syscall exit codes and provide a rustacean `io::Result`. -pub struct SyscallReturnCode(pub c_int); +#[derive(Debug)] +pub struct SyscallReturnCode + Eq = c_int>(pub T); -impl SyscallReturnCode { +impl + Eq> SyscallReturnCode { /// Returns the last OS error if value is -1 or Ok(value) otherwise. - pub fn into_result(self) -> std::io::Result { - if self.0 == -1 { + pub fn into_result(self) -> std::io::Result { + if self.0 == T::from(-1) { Err(std::io::Error::last_os_error()) } else { Ok(self.0) @@ -46,5 +47,14 @@ mod tests { syscall_code = SyscallReturnCode(-1); assert!(syscall_code.into_empty_result().is_err()); + + let mut syscall_code_long = SyscallReturnCode(1i64); + match syscall_code_long.into_result() { + Ok(_value) => (), + _ => unreachable!(), + } + + syscall_code_long = SyscallReturnCode(-1i64); + assert!(syscall_code_long.into_result().is_err()); } } diff --git a/src/tempfile.rs b/src/tempfile.rs index 7d70f66..de663c9 100644 --- a/src/tempfile.rs +++ b/src/tempfile.rs @@ -39,6 +39,7 @@ use crate::errno::{errno_result, Error, Result}; /// Wrapper for working with temporary files. /// /// The file will be maintained for the lifetime of the `TempFile` object. +#[derive(Debug)] pub struct TempFile { path: PathBuf, file: Option, @@ -60,22 +61,21 @@ impl TempFile { let mut os_fname = prefix.as_ref().to_os_string(); os_fname.push("XXXXXX"); - let raw_fname = match CString::new(os_fname.as_bytes()) { - Ok(c_string) => c_string.into_raw(), - Err(_) => return Err(Error::new(libc::EINVAL)), - }; + let c_tempname = CString::new(os_fname.as_bytes()).map_err(|_| Error::new(libc::EINVAL))?; + let raw_tempname = c_tempname.into_raw(); - // SAFETY: Safe because `raw_fname` originates from CString::into_raw, meaning - // it is a pointer to a nul-terminated sequence of characters. - let fd = unsafe { libc::mkstemp(raw_fname) }; - if fd == -1 { - return errno_result(); - } + // SAFETY: Safe because `c_tempname` is a null-terminated string, as it originates from + // `CString::into_raw`. + let ret = unsafe { libc::mkstemp(raw_tempname) }; + + // SAFETY: `raw_tempname` originates from `CString::into_raw`. + let c_tempname = unsafe { CString::from_raw(raw_tempname) }; + + let fd = match ret { + -1 => return errno_result(), + _ => ret, + }; - // SAFETY: raw_fname originates from a call to CString::into_raw. The length - // of the string has not changed, as mkstemp returns a valid file name, and - // '\0' cannot be part of a valid filename. - let c_tempname = unsafe { CString::from_raw(raw_fname) }; let os_tempname = OsStr::from_bytes(c_tempname.as_bytes()); // SAFETY: Safe because we checked `fd != -1` above and we uniquely own the file diff --git a/src/unix/tempdir.rs b/src/unix/tempdir.rs index 101d35a..980fc88 100644 --- a/src/unix/tempdir.rs +++ b/src/unix/tempdir.rs @@ -16,6 +16,7 @@ use crate::errno::{errno_result, Error, Result}; /// Wrapper over a temporary directory. /// /// The directory will be maintained for the lifetime of the `TempDir` object. +#[derive(Debug)] pub struct TempDir { path: PathBuf, } -- cgit v1.2.3