From def1d54aa607d8e34e24f2e4a7314ca0acdb298b Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Thu, 16 Jul 2020 13:44:42 +0200 Subject: [PATCH] various clippy lint fixes Signed-off-by: Wolfgang Bumiller --- proxmox-api-macro/src/api.rs | 2 +- proxmox-api-macro/src/api/method.rs | 6 ++---- proxmox-api-macro/src/api/structs.rs | 28 +++++++++++++--------------- proxmox/src/tools/io/read.rs | 20 ++++++++++++++++++++ proxmox/src/tools/io/write.rs | 15 +++++++++++++++ proxmox/src/tools/mmap.rs | 7 +++++++ proxmox/src/tools/serde.rs | 2 +- proxmox/src/tools/time.rs | 4 ++-- proxmox/src/tools/vec.rs | 6 ++++++ proxmox/src/tools/vec/byte_vec.rs | 12 ++++++++++++ 10 files changed, 79 insertions(+), 23 deletions(-) diff --git a/proxmox-api-macro/src/api.rs b/proxmox-api-macro/src/api.rs index 6bd9f604..165dbf00 100644 --- a/proxmox-api-macro/src/api.rs +++ b/proxmox-api-macro/src/api.rs @@ -200,7 +200,7 @@ impl Schema { } pub fn add_default_property(&mut self, key: &str, value: syn::Expr) { - if !self.find_schema_property(key).is_some() { + if self.find_schema_property(key).is_none() { self.properties.push((Ident::new(key, Span::call_site()), value)); } } diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs index cdadb6f0..da786533 100644 --- a/proxmox-api-macro/src/api/method.rs +++ b/proxmox-api-macro/src/api/method.rs @@ -212,10 +212,8 @@ fn handle_function_signature( // try to infer the type in the schema if it is not specified explicitly: let is_option = util::infer_type(schema, &*pat_type.ty)?; let has_default = schema.find_schema_property("default").is_some(); - if !is_option && *optional { - if !has_default { - bail!(pat_type => "optional types need a default or be an Option"); - } + if !is_option && *optional && !has_default { + bail!(pat_type => "optional types need a default or be an Option"); } if has_default && !*optional { bail!(pat_type => "non-optional parameter cannot have a default"); diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs index f6167753..0c581162 100644 --- a/proxmox-api-macro/src/api/structs.rs +++ b/proxmox-api-macro/src/api/structs.rs @@ -94,23 +94,21 @@ fn handle_newtype_struct( // create "linked" schemas. We cannot do this purely via the macro. let mut schema: Schema = attribs.try_into()?; - match schema.item { - SchemaItem::Inferred(_span) => { - // The schema has no `type` and we failed to guess it. Infer it from the contained - // field! + if let SchemaItem::Inferred(_span) = schema.item { + // The schema has no `type` and we failed to guess it. Infer it from the contained field! - let fields = match &stru.fields { - syn::Fields::Unnamed(fields) => &fields.unnamed, - _ => panic!("handle_unit_struct on non-unit struct"), // `handle_struct()` verified this! - }; - // this is also part of `handle_struct()`'s verification! - assert_eq!(fields.len(), 1, "handle_unit_struct needs a struct with exactly 1 field"); + let fields = match &stru.fields { + syn::Fields::Unnamed(fields) => &fields.unnamed, - // Now infer the type information: - util::infer_type(&mut schema, &fields[0].ty)?; - } - _ => (), - }; + // `handle_struct()` verified this! + _ => panic!("handle_unit_struct on non-unit struct"), + }; + // this is also part of `handle_struct()`'s verification! + assert_eq!(fields.len(), 1, "handle_unit_struct needs a struct with exactly 1 field"); + + // Now infer the type information: + util::infer_type(&mut schema, &fields[0].ty)?; + } get_struct_description(&mut schema, &stru)?; diff --git a/proxmox/src/tools/io/read.rs b/proxmox/src/tools/io/read.rs index 734296a8..1bd331e6 100644 --- a/proxmox/src/tools/io/read.rs +++ b/proxmox/src/tools/io/read.rs @@ -105,6 +105,11 @@ pub trait ReadExt { /// # } /// ``` /// + /// # Safety + /// + /// This should only used for types with a defined storage representation, usually + /// `#[repr(C)]`, otherwise the results may be inconsistent. + /// /// [`Endian`]: https://docs.rs/endian_trait/0.6/endian_trait/trait.Endian.html unsafe fn read_host_value(&mut self) -> io::Result; @@ -137,6 +142,11 @@ pub trait ReadExt { /// # } /// ``` /// + /// # Safety + /// + /// This should only used for types with a defined storage representation, usually + /// `#[repr(C)]`, otherwise the results may be inconsistent. + /// /// [`Endian`]: https://docs.rs/endian_trait/0.6/endian_trait/trait.Endian.html unsafe fn read_le_value(&mut self) -> io::Result; @@ -169,6 +179,11 @@ pub trait ReadExt { /// # } /// ``` /// + /// # Safety + /// + /// This should only used for types with a defined storage representation, usually + /// `#[repr(C)]`, otherwise the results may be inconsistent. + /// /// [`Endian`]: https://docs.rs/endian_trait/0.6/endian_trait/trait.Endian.html unsafe fn read_be_value(&mut self) -> io::Result; @@ -201,6 +216,11 @@ pub trait ReadExt { /// # } /// # code(&input[..]).unwrap(); /// ``` + /// + /// # Safety + /// + /// This should only used for types with a defined storage representation, usually + /// `#[repr(C)]`, otherwise the results may be inconsistent. unsafe fn read_host_value_boxed(&mut self) -> io::Result>; } diff --git a/proxmox/src/tools/io/write.rs b/proxmox/src/tools/io/write.rs index 21ec0766..3ed3562f 100644 --- a/proxmox/src/tools/io/write.rs +++ b/proxmox/src/tools/io/write.rs @@ -70,6 +70,11 @@ pub trait WriteExt { /// # } /// ``` /// + /// # Safety + /// + /// This should only used for types with a defined storage representation, usually + /// `#[repr(C)]`, otherwise the results may be inconsistent. + /// /// [`Endian`]: https://docs.rs/endian_trait/0.6/endian_trait/trait.Endian.html unsafe fn write_host_value(&mut self, value: T) -> io::Result<()>; @@ -108,6 +113,11 @@ pub trait WriteExt { /// # } /// ``` /// + /// # Safety + /// + /// This should only used for types with a defined storage representation, usually + /// `#[repr(C)]`, otherwise the results may be inconsistent. + /// /// [`Endian`]: https://docs.rs/endian_trait/0.6/endian_trait/trait.Endian.html unsafe fn write_le_value(&mut self, value: T) -> io::Result<()>; @@ -146,6 +156,11 @@ pub trait WriteExt { /// # } /// ``` /// + /// # Safety + /// + /// This should only used for types with a defined storage representation, usually + /// `#[repr(C)]`, otherwise the results may be inconsistent. + /// /// [`Endian`]: https://docs.rs/endian_trait/0.6/endian_trait/trait.Endian.html unsafe fn write_be_value(&mut self, value: T) -> io::Result<()>; } diff --git a/proxmox/src/tools/mmap.rs b/proxmox/src/tools/mmap.rs index dd2347aa..214f4b6e 100644 --- a/proxmox/src/tools/mmap.rs +++ b/proxmox/src/tools/mmap.rs @@ -17,6 +17,11 @@ unsafe impl Send for Mmap where T: Send {} unsafe impl Sync for Mmap where T: Sync {} impl Mmap { + /// Map a file into memory. + /// + /// # Safety + /// + /// `fd` must refer to a valid file descriptor. pub unsafe fn map_fd( fd: RawFd, ofs: u64, @@ -25,6 +30,8 @@ impl Mmap { flags: mman::MapFlags, ) -> io::Result { let byte_len = count * mem::size_of::(); + // libc::size_t vs usize + #[allow(clippy::useless_conversion)] let data = mman::mmap( ptr::null_mut(), libc::size_t::try_from(byte_len).map_err(io_err_other)?, diff --git a/proxmox/src/tools/serde.rs b/proxmox/src/tools/serde.rs index 4e6216f0..9fb28b10 100644 --- a/proxmox/src/tools/serde.rs +++ b/proxmox/src/tools/serde.rs @@ -119,7 +119,7 @@ pub mod string_as_base64 { use base64; use serde::{Deserialize, Deserializer, Serializer}; - pub fn serialize(data: &String, serializer: S) -> Result + pub fn serialize(data: &str, serializer: S) -> Result where S: Serializer, { diff --git a/proxmox/src/tools/time.rs b/proxmox/src/tools/time.rs index af4bf59c..7fdbc1ee 100644 --- a/proxmox/src/tools/time.rs +++ b/proxmox/src/tools/time.rs @@ -50,7 +50,7 @@ pub fn localtime(epoch: i64) -> Result { let mut result = new_libc_tm(); unsafe { - if libc::localtime_r(&epoch, &mut result) == std::ptr::null_mut() { + if libc::localtime_r(&epoch, &mut result).is_null() { bail!("libc::localtime failed for '{}'", epoch); } } @@ -64,7 +64,7 @@ pub fn gmtime(epoch: i64) -> Result { let mut result = new_libc_tm(); unsafe { - if libc::gmtime_r(&epoch, &mut result) == std::ptr::null_mut() { + if libc::gmtime_r(&epoch, &mut result).is_null() { bail!("libc::gmtime failed for '{}'", epoch); } } diff --git a/proxmox/src/tools/vec.rs b/proxmox/src/tools/vec.rs index a74f62be..8ea343c3 100644 --- a/proxmox/src/tools/vec.rs +++ b/proxmox/src/tools/vec.rs @@ -44,6 +44,12 @@ pub use byte_vec::*; /// v.set_len(len); /// } /// ``` +/// +/// # Safety +/// +/// It's generally not unsafe to use this method, but the contents are uninitialized, and since +/// this does not return a `MaybeUninit` type to track the initialization state, this is simply +/// marked as unsafe for good measure. #[inline] pub unsafe fn uninitialized(len: usize) -> Vec { let mut out = Vec::with_capacity(len); diff --git a/proxmox/src/tools/vec/byte_vec.rs b/proxmox/src/tools/vec/byte_vec.rs index ad98dcf9..e983f96d 100644 --- a/proxmox/src/tools/vec/byte_vec.rs +++ b/proxmox/src/tools/vec/byte_vec.rs @@ -64,6 +64,12 @@ pub trait ByteVecExt { /// file.append_to_vec(&mut buffer, 1024)?; /// ``` /// + /// # Safety + /// + /// When increasing the size, the new contents are uninitialized and have nothing to do with + /// the previously contained content. Since we cannot track this state through the type system, + /// this method is marked as an unsafe API for good measure. + /// /// [`ReadExt`]: crate::io::ReadExt unsafe fn grow_uninitialized(&mut self, more: usize) -> &mut [u8]; @@ -77,6 +83,12 @@ pub trait ByteVecExt { /// } /// } /// ``` + /// + /// # Safety + /// + /// When increasing the size, the new contents are uninitialized and have nothing to do with + /// the previously contained content. Since we cannot track this state through the type system, + /// this method is marked as an unsafe API for good measure. unsafe fn resize_uninitialized(&mut self, total: usize); }