From: Wolfgang Bumiller Date: Thu, 16 Jul 2020 09:21:13 +0000 (+0200) Subject: more clippy lints X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=1b25fc08208c7d3694f63105c96f0f14a3966b9a;p=pxar.git more clippy lints Signed-off-by: Wolfgang Bumiller --- diff --git a/src/accessor/aio.rs b/src/accessor/aio.rs index 8e20dd2..a1aaa08 100644 --- a/src/accessor/aio.rs +++ b/src/accessor/aio.rs @@ -92,7 +92,7 @@ impl Accessor { } /// Open a directory handle to the root of the pxar archive. - pub async fn open_root_ref<'a>(&'a self) -> io::Result> { + pub async fn open_root_ref(&self) -> io::Result> { Ok(Directory::new(self.inner.open_root_ref().await?)) } @@ -117,11 +117,21 @@ impl Accessor { } /// Allow opening a directory at a specified offset. + /// + /// # Safety + /// + /// This should only be used with offsets known to point to the end of a directory, otherwise + /// this usually fails, unless the data otherwise happens to look like a valid directory. pub async unsafe fn open_dir_at_end(&self, offset: u64) -> io::Result> { Ok(Directory::new(self.inner.open_dir_at_end(offset).await?)) } /// Allow opening a regular file from a specified range. + /// + /// # Safety + /// + /// Should only be used with `entry_range_info`s originating from the same archive, otherwise + /// the result will be undefined and likely fail (or contain unexpected data). pub async unsafe fn open_file_at_range( &self, entry_range_info: &accessor::EntryRangeInfo, @@ -132,6 +142,11 @@ impl Accessor { } /// Allow opening arbitrary contents from a specific range. + /// + /// # Safety + /// + /// This will provide a reader over an arbitrary range of the archive file, so unless this + /// comes from a actual file entry data, the contents might not make much sense. pub unsafe fn open_contents_at_range(&self, range: Range) -> FileContents { FileContents { inner: self.inner.open_contents_at_range(range), @@ -186,7 +201,7 @@ impl Directory { } /// Get an iterator over the directory's contents. - pub fn read_dir<'a>(&'a self) -> ReadDir<'a, T> { + pub fn read_dir(&self) -> ReadDir { ReadDir { inner: self.inner.read_dir(), } @@ -313,12 +328,18 @@ impl<'a, T: Clone + ReadAt> DirEntry<'a, T> { } } +/// File content read future result. +struct ReadResult { + len: usize, + buffer: Vec, +} + /// A reader for file contents. pub struct FileContents { inner: accessor::FileContentsImpl, at: u64, buffer: Vec, - future: Option)>> + 'static>>>, + future: Option> + 'static>>>, } // We lose `Send` via the boxed trait object and don't want to force the trait object to @@ -343,10 +364,10 @@ impl FileContents { util::scale_read_buffer(&mut buffer, dest.len()); let reader: accessor::FileContentsImpl = this.inner.clone(); let at = this.at; - let future: Pin)>>>> = + let future: Pin>>> = Box::pin(async move { - let got = reader.read_at(&mut buffer, at).await?; - io::Result::Ok((got, buffer)) + let len = reader.read_at(&mut buffer, at).await?; + io::Result::Ok(ReadResult { len, buffer }) }); // This future has the lifetime from T. Self also has this lifetime and we // store this in a pinned self. T maybe a reference with a non-'static life @@ -360,7 +381,7 @@ impl FileContents { return Poll::Pending; } Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), - Poll::Ready(Ok((got, buffer))) => { + Poll::Ready(Ok(ReadResult { len: got, buffer })) => { this.buffer = buffer; this.at += got as u64; let len = got.min(dest.len()); diff --git a/src/accessor/mod.rs b/src/accessor/mod.rs index f998482..0ab03a6 100644 --- a/src/accessor/mod.rs +++ b/src/accessor/mod.rs @@ -202,7 +202,7 @@ impl AccessorImpl { self.size } - pub async fn open_root_ref<'a>(&'a self) -> io::Result> { + pub async fn open_root_ref(&self) -> io::Result> { DirectoryImpl::open_at_end( &self.input as &dyn ReadAt, self.size, @@ -218,7 +218,7 @@ impl AccessorImpl { ) { let new_caches = Arc::new(Caches { gbt_cache: cache, - ..*self.caches + //..*self.caches }); self.caches = new_caches; } @@ -436,7 +436,6 @@ impl DirectoryImpl { len * size_of::(), ); read_exact_at(&self.input, slice, self.table_offset()).await?; - drop(slice); } Ok(Arc::from(data)) } @@ -608,6 +607,8 @@ impl DirectoryImpl { } } + // while clippy is technically right about this, the compiler won't accept it (yet) + #[allow(clippy::needless_lifetimes)] async fn get_cursor<'a>(&'a self, index: usize) -> io::Result> { let entry = &self.table[index]; let file_goodbye_ofs = entry.offset; @@ -881,12 +882,18 @@ impl ReadAt for FileContentsImpl { } } +/// File content read future result. +struct ReadResult { + len: usize, + buffer: Vec, +} + #[doc(hidden)] pub struct SeqReadAtAdapter { input: T, range: Range, buffer: Vec, - future: Option)>> + 'static>>>, + future: Option> + 'static>>>, } // We lose `Send` via the boxed trait object and don't want to force the trait object to @@ -943,10 +950,10 @@ impl decoder::SeqRead for SeqReadAtAdapter { let reader = &this.input; let at = this.range.start; - let future: Pin)>>>> = + let future: Pin>>> = Box::pin(async move { - let got = reader.read_at(&mut buffer, at).await?; - io::Result::Ok((got, buffer)) + let len = reader.read_at(&mut buffer, at).await?; + io::Result::Ok(ReadResult { len, buffer }) }); // Ditch the self-reference life-time now: this.future = Some(unsafe { mem::transmute(future) }); @@ -957,7 +964,7 @@ impl decoder::SeqRead for SeqReadAtAdapter { return Poll::Pending; } Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), - Poll::Ready(Ok((got, buffer))) => { + Poll::Ready(Ok(ReadResult { len: got, buffer })) => { this.buffer = buffer; this.range.start += got as u64; let len = got.min(dest.len()); diff --git a/src/accessor/sync.rs b/src/accessor/sync.rs index 50334b9..15c99c4 100644 --- a/src/accessor/sync.rs +++ b/src/accessor/sync.rs @@ -108,6 +108,11 @@ impl Accessor { } /// Allow opening a directory at a specified offset. + /// + /// # Safety + /// + /// This should only be used with offsets known to point to the end of a directory, otherwise + /// this usually fails, unless the data otherwise happens to look like a valid directory. pub unsafe fn open_dir_at_end(&self, offset: u64) -> io::Result> { Ok(Directory::new(poll_result_once( self.inner.open_dir_at_end(offset), @@ -115,6 +120,11 @@ impl Accessor { } /// Allow opening a regular file from a specified range. + /// + /// # Safety + /// + /// Should only be used with `entry_range_info`s originating from the same archive, otherwise + /// the result will be undefined and likely fail (or contain unexpected data). pub unsafe fn open_file_at_range( &self, entry_range_info: &accessor::EntryRangeInfo, @@ -125,6 +135,11 @@ impl Accessor { } /// Allow opening arbitrary contents from a specific range. + /// + /// # Safety + /// + /// This will provide a reader over an arbitrary range of the archive file, so unless this + /// comes from a actual file entry data, the contents might not make much sense. pub unsafe fn open_contents_at_range(&self, range: Range) -> FileContents { FileContents { inner: self.inner.open_contents_at_range(range), @@ -243,7 +258,7 @@ impl Directory { } /// Get an iterator over the directory's contents. - pub fn read_dir<'a>(&'a self) -> ReadDir<'a, T> { + pub fn read_dir(&self) -> ReadDir { ReadDir { inner: self.inner.read_dir(), } diff --git a/src/decoder/aio.rs b/src/decoder/aio.rs index e212c5e..e7152b3 100644 --- a/src/decoder/aio.rs +++ b/src/decoder/aio.rs @@ -57,6 +57,9 @@ impl Decoder { Self { inner } } + // I would normally agree with clippy, but this is async and we can at most implement Stream, + // which we do with feature flags... + #[allow(clippy::should_implement_trait)] /// If this is a directory entry, get the next item inside the directory. pub async fn next(&mut self) -> Option> { self.inner.next_do().await.transpose() @@ -87,6 +90,7 @@ mod stream { /// /// As long as streams are poll-based this wrapper is required to turn `async fn next()` into /// `Stream`'s `poll_next()` interface. + #[allow(clippy::type_complexity)] // yeah no pub struct DecoderStream { inner: super::Decoder, future: Option>>>>>, diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 1bfbcb9..9410942 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -293,7 +293,7 @@ impl DecoderImpl { } } - pub fn content_reader<'a>(&'a mut self) -> Option> { + pub fn content_reader(&mut self) -> Option> { if let State::InPayload { offset } = &mut self.state { Some(Contents::new( &mut self.input, diff --git a/src/decoder/sync.rs b/src/decoder/sync.rs index b0a9f76..a7afda1 100644 --- a/src/decoder/sync.rs +++ b/src/decoder/sync.rs @@ -54,6 +54,9 @@ impl Decoder { Self { inner } } + // I would normally agree with clippy, but this here is to be consistent with the async + // counterpart, and we *do* implement Iterator as well, so that's fine! + #[allow(clippy::should_implement_trait)] /// If this is a directory entry, get the next item inside the directory. pub fn next(&mut self) -> Option> { poll_result_once(self.inner.next_do()).transpose() diff --git a/src/encoder/aio.rs b/src/encoder/aio.rs index ab20a4a..e2d5461 100644 --- a/src/encoder/aio.rs +++ b/src/encoder/aio.rs @@ -297,7 +297,7 @@ mod futures_writer { fn poll_close(self: Pin<&mut Self>, cx: &mut Context) -> Poll> { let this = unsafe { self.get_unchecked_mut() }; match this.inner.as_mut() { - None => return Poll::Ready(Ok(())), + None => Poll::Ready(Ok(())), Some(inner) => { ready!(unsafe { Pin::new_unchecked(inner).poll_close(cx) })?; this.inner = None; @@ -368,7 +368,7 @@ mod tokio_writer { fn poll_close(self: Pin<&mut Self>, cx: &mut Context) -> Poll> { let this = unsafe { self.get_unchecked_mut() }; match this.inner.as_mut() { - None => return Poll::Ready(Ok(())), + None => Poll::Ready(Ok(())), Some(inner) => { ready!(unsafe { Pin::new_unchecked(inner).poll_shutdown(cx) })?; this.inner = None; diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index f100939..c098dcb 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -506,7 +506,7 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> { entry_offset, files_offset, file_offset: Some(file_offset), - file_hash: file_hash, + file_hash, ..Default::default() }, parent: Some(&mut self.state), @@ -605,7 +605,7 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> { seq_write_pxar_struct_entry( &mut self.output, format::PXAR_QUOTA_PROJID, - quota_project_id.clone(), + *quota_project_id, ) .await } diff --git a/src/format/mod.rs b/src/format/mod.rs index e9e8d1b..2a7d377 100644 --- a/src/format/mod.rs +++ b/src/format/mod.rs @@ -327,15 +327,13 @@ impl From<&std::fs::Metadata> for Entry { let file_type = meta.file_type(); let mode = this.mode; - let this = if file_type.is_dir() { + if file_type.is_dir() { this.mode(mode | mode::IFDIR) } else if file_type.is_symlink() { this.mode(mode | mode::IFLNK) } else { this.mode(mode | mode::IFREG) - }; - - this + } } } diff --git a/src/lib.rs b/src/lib.rs index 1e88748..5d1b781 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,11 +64,8 @@ impl From for Metadata { impl From<&std::fs::Metadata> for Metadata { fn from(meta: &std::fs::Metadata) -> Metadata { - let this = Self::from(Stat::from(meta)); - - // FIXME: fill the remaining metadata - - this + // NOTE: fill the remaining metadata via feature flags? + Self::from(Stat::from(meta)) } } @@ -387,7 +384,7 @@ impl Entry { /// Convenience method to get just the file name portion of the current path. #[inline] pub fn file_name(&self) -> &OsStr { - self.path.file_name().unwrap_or(OsStr::new("")) + self.path.file_name().unwrap_or_else(|| OsStr::new("")) } /// Get the file metadata.