From 29add10ca7e1cc4cfed919bd1d9a13f43b7d52f7 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:06:42 +0200 Subject: [PATCH 1/9] Fix manual non_exhaustive pattern --- src/decoder/mod.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 0f91d23..8993649 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -206,6 +206,7 @@ pub enum ChunkType { /// Decoding limits #[derive(Clone, Debug)] +#[non_exhaustive] pub struct Limits { /// The maximum size of any `DecodingResult` in bytes, the default is /// 256MiB. If the entire image is decoded at once, then this will @@ -218,10 +219,6 @@ pub struct Limits { /// Maximum size for intermediate buffer which may be used to limit the amount of data read per /// segment even if the entire image is decoded at once. pub intermediate_buffer_size: usize, - /// The purpose of this is to prevent all the fields of the struct from - /// being public, as this would make adding new fields a major version - /// bump. - _non_exhaustive: (), } impl Limits { @@ -234,10 +231,9 @@ impl Limits { /// naturally, the machine running the program does not have infinite memory. pub fn unlimited() -> Limits { Limits { - decoding_buffer_size: usize::max_value(), - ifd_value_size: usize::max_value(), - intermediate_buffer_size: usize::max_value(), - _non_exhaustive: (), + decoding_buffer_size: usize::MAX, + ifd_value_size: usize::MAX, + intermediate_buffer_size: usize::MAX, } } } @@ -248,7 +244,6 @@ impl Default for Limits { decoding_buffer_size: 256 * 1024 * 1024, intermediate_buffer_size: 128 * 1024 * 1024, ifd_value_size: 1024 * 1024, - _non_exhaustive: (), } } } From 2942f9de972b463ecb17014a02466f261d4c26dc Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:07:08 +0200 Subject: [PATCH 2/9] Mark f16 bytecast as expected unused --- src/bytecast.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/bytecast.rs b/src/bytecast.rs index 88d562c..9eeccfc 100644 --- a/src/bytecast.rs +++ b/src/bytecast.rs @@ -10,6 +10,9 @@ //! the unsafe code guidelines). //! //! TODO: Would like to use std-lib here. +// Until we implement predictors for f16, we don't need f16_as_ne_bytes. (Due to the macro we do +// not apply this directly to the functions). And rust-version is not new enough for `expect`. +#![allow(dead_code)] use std::{mem, slice}; use half::f16; From ca74c03520fa04d991dcc13918114da476f89ad5 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:08:15 +0200 Subject: [PATCH 3/9] Fix straightforward cast/borrow lints --- src/decoder/ifd.rs | 2 +- src/decoder/image.rs | 10 +++------- src/encoder/mod.rs | 6 +++--- tests/predict.rs | 2 +- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/decoder/ifd.rs b/src/decoder/ifd.rs index ce9c8e1..0b68d56 100644 --- a/src/decoder/ifd.rs +++ b/src/decoder/ifd.rs @@ -445,7 +445,7 @@ impl Entry { // 2b: the value is at most 4 bytes or doesn't fit in the offset field. return Ok(match self.type_ { Type::BYTE => Byte(self.offset[0]), - Type::SBYTE => SignedByte(i8::from(self.offset[0] as i8)), + Type::SBYTE => SignedByte(self.offset[0] as i8), Type::UNDEFINED => Byte(self.offset[0]), Type::SHORT => Short(self.r(bo).read_u16()?), Type::SSHORT => SignedShort(self.r(bo).read_i16()?), diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 49510a3..e3dc31f 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -643,11 +643,11 @@ impl Image { self.jpeg_tables.as_deref().map(|a| &**a), )?; - if output_row_stride == chunk_row_bytes as usize { + if output_row_stride == chunk_row_bytes { let tile = &mut buf[..chunk_row_bytes * data_dims.1 as usize]; reader.read_exact(tile)?; - for row in tile.chunks_mut(chunk_row_bytes as usize) { + for row in tile.chunks_mut(chunk_row_bytes) { super::fix_endianness_and_predict( row, color_type.bit_depth(), @@ -678,11 +678,7 @@ impl Image { } } } else { - for (_i, row) in buf - .chunks_mut(output_row_stride) - .take(data_dims.1 as usize) - .enumerate() - { + for row in buf.chunks_mut(output_row_stride).take(data_dims.1 as usize) { let row = &mut row[..data_row_bytes]; reader.read_exact(row)?; diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 1c95821..717f1f0 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -296,7 +296,7 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { } let entry = Self::write_value( - &mut self.writer, + self.writer, &DirectoryEntry { data_type: ::FIELD_TYPE, count: value.count().try_into()?, @@ -370,14 +370,14 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> { if bytes.len() > in_entry_bytes { let offset = writer.offset(); - writer.write_bytes(&bytes)?; + writer.write_bytes(bytes)?; let offset = K::convert_offset(offset)?; offset_bytes[..offset.bytes()].copy_from_slice(&offset.data()); } else { // Note: we have indicated our native byte order in the header, hence this // corresponds to our byte order no matter the value type. - offset_bytes[..bytes.len()].copy_from_slice(&bytes); + offset_bytes[..bytes.len()].copy_from_slice(bytes); } // Undoing some hidden type. Offset is either u32 or u64. Due to the trait API being public diff --git a/tests/predict.rs b/tests/predict.rs index a61eb6c..59bb79c 100644 --- a/tests/predict.rs +++ b/tests/predict.rs @@ -1,7 +1,7 @@ extern crate tiff; use tiff::decoder::{Decoder, DecodingResult}; -use tiff::encoder::{colortype, Compression, Predictor, TiffEncoder}; +use tiff::encoder::{colortype, Predictor, TiffEncoder}; use tiff::ColorType; use std::fs::File; From 23672b2490dc544eefd37b538072448abc9441f6 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:09:19 +0200 Subject: [PATCH 4/9] Switch to div_ceil for rounding implementation --- src/decoder/image.rs | 8 ++++---- src/decoder/mod.rs | 7 ++++--- src/encoder/mod.rs | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index e3dc31f..262f333 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -29,10 +29,10 @@ pub(crate) struct TileAttributes { impl TileAttributes { pub fn tiles_across(&self) -> usize { - (self.image_width + self.tile_width - 1) / self.tile_width + self.image_width.div_ceil(self.tile_width) } pub fn tiles_down(&self) -> usize { - (self.image_height + self.tile_length - 1) / self.tile_length + self.image_height.div_ceil(self.tile_length) } fn padding_right(&self) -> usize { (self.tile_width - self.image_width % self.tile_width) % self.tile_width @@ -625,12 +625,12 @@ impl Image { let chunk_row_bits = (u64::from(chunk_dims.0) * u64::from(self.bits_per_sample)) .checked_mul(samples as u64) .ok_or(TiffError::LimitsExceeded)?; - let chunk_row_bytes: usize = ((chunk_row_bits + 7) / 8).try_into()?; + let chunk_row_bytes: usize = chunk_row_bits.div_ceil(8).try_into()?; let data_row_bits = (u64::from(data_dims.0) * u64::from(self.bits_per_sample)) .checked_mul(samples as u64) .ok_or(TiffError::LimitsExceeded)?; - let data_row_bytes: usize = ((data_row_bits + 7) / 8).try_into()?; + let data_row_bytes: usize = data_row_bits.div_ceil(8).try_into()?; // TODO: Should these return errors instead? assert!(output_row_stride >= data_row_bytes); diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 8993649..ae0ec7a 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -856,7 +856,8 @@ impl Decoder { let row_samples = if bits_per_sample >= 8 { width } else { - ((((width as u64) * bits_per_sample as u64) + 7) / 8) + ((width as u64) * bits_per_sample as u64) + .div_ceil(8) .try_into() .map_err(|_| TiffError::LimitsExceeded)? }; @@ -955,12 +956,12 @@ impl Decoder { let output_row_bits = (width as u64 * self.image.bits_per_sample as u64) .checked_mul(samples as u64) .ok_or(TiffError::LimitsExceeded)?; - let output_row_stride: usize = ((output_row_bits + 7) / 8).try_into()?; + let output_row_stride: usize = output_row_bits.div_ceil(8).try_into()?; let chunk_row_bits = (chunk_dimensions.0 as u64 * self.image.bits_per_sample as u64) .checked_mul(samples as u64) .ok_or(TiffError::LimitsExceeded)?; - let chunk_row_bytes: usize = ((chunk_row_bits + 7) / 8).try_into()?; + let chunk_row_bytes: usize = chunk_row_bits.div_ceil(8).try_into()?; let chunks_across = ((width - 1) / chunk_dimensions.0 + 1) as usize; diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 717f1f0..9fc7b7d 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -532,11 +532,11 @@ impl<'a, W: 'a + Write + Seek, T: ColorType, K: TiffKind> ImageEncoder<'a, W, T, let rows_per_strip = { match compression.tag() { CompressionMethod::PackBits => 1, // Each row must be packed separately. Do not compress across row boundaries - _ => (1_000_000 + row_bytes - 1) / row_bytes, + _ => 1_000_000_u64.div_ceil(row_bytes), } }; - let strip_count = (u64::from(height) + rows_per_strip - 1) / rows_per_strip; + let strip_count = u64::from(height).div_ceil(rows_per_strip); encoder.write_tag(Tag::ImageWidth, width)?; encoder.write_tag(Tag::ImageLength, height)?; @@ -705,7 +705,7 @@ impl<'a, W: 'a + Write + Seek, T: ColorType, K: TiffKind> ImageEncoder<'a, W, T, self.encoder.write_tag(Tag::RowsPerStrip, value)?; let value: u64 = value as u64; - self.strip_count = (self.height as u64 + value - 1) / value; + self.strip_count = (self.height as u64).div_ceil(value); self.rows_per_strip = value; Ok(()) From f6a762b72c8650067b815d11a43492756444807e Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:09:55 +0200 Subject: [PATCH 5/9] Upgrade usage of format! and Default --- src/encoder/compression/deflate.rs | 8 ++------ src/encoder/tiff_value.rs | 2 +- tests/decode_images.rs | 6 +++--- tests/encode_images.rs | 2 +- tests/encode_images_with_compression.rs | 4 ++-- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/encoder/compression/deflate.rs b/src/encoder/compression/deflate.rs index a8a052e..3e5d528 100644 --- a/src/encoder/compression/deflate.rs +++ b/src/encoder/compression/deflate.rs @@ -11,21 +11,17 @@ pub struct Deflate { /// It allows trading compression ratio for compression speed. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] #[non_exhaustive] +#[derive(Default)] pub enum DeflateLevel { /// The fastest possible compression mode. Fast = 1, /// The conserative choice between speed and ratio. + #[default] Balanced = 6, /// The best compression available with Deflate. Best = 9, } -impl Default for DeflateLevel { - fn default() -> Self { - DeflateLevel::Balanced - } -} - impl Deflate { /// Create a new deflate compressor with a specific level of compression. pub fn with_level(level: DeflateLevel) -> Self { diff --git a/src/encoder/tiff_value.rs b/src/encoder/tiff_value.rs index 43653f4..c9b7af9 100644 --- a/src/encoder/tiff_value.rs +++ b/src/encoder/tiff_value.rs @@ -450,7 +450,7 @@ impl TiffValue for str { } } -impl<'a, T: TiffValue + ?Sized> TiffValue for &'a T { +impl TiffValue for &'_ T { const BYTE_LEN: u8 = T::BYTE_LEN; const FIELD_TYPE: Type = T::FIELD_TYPE; diff --git a/tests/decode_images.rs b/tests/decode_images.rs index 5a62279..a459c86 100644 --- a/tests/decode_images.rs +++ b/tests/decode_images.rs @@ -348,7 +348,7 @@ fn test_div_zero() { match err { TiffError::FormatError(TiffFormatError::StripTileTagConflict) => {} - unexpected => panic!("Unexpected error {}", unexpected), + unexpected => panic!("Unexpected error {unexpected}"), } } @@ -366,7 +366,7 @@ fn test_too_many_value_bytes() { match error { tiff::TiffError::LimitsExceeded => {} - unexpected => panic!("Unexpected error {}", unexpected), + unexpected => panic!("Unexpected error {unexpected}"), } } @@ -497,7 +497,7 @@ fn timeout() { match error { TiffError::FormatError(TiffFormatError::CycleInOffsets) => {} - e => panic!("Unexpected error {:?}", e), + e => panic!("Unexpected error {e:?}"), } } diff --git a/tests/encode_images.rs b/tests/encode_images.rs index af9f610..c183543 100644 --- a/tests/encode_images.rs +++ b/tests/encode_images.rs @@ -621,7 +621,7 @@ fn test_rows_per_strip() { let img2 = [i; 2 * 100]; match decoder.read_chunk(i as u32).unwrap() { DecodingResult::U8(data) => assert_eq!(&img2[..], &data[..]), - other => panic!("Incorrect strip type {:?}", other), + other => panic!("Incorrect strip type {other:?}"), } } } diff --git a/tests/encode_images_with_compression.rs b/tests/encode_images_with_compression.rs index 9b8a92f..a355fb1 100644 --- a/tests/encode_images_with_compression.rs +++ b/tests/encode_images_with_compression.rs @@ -116,7 +116,7 @@ fn encode_decode_with_compression(compression: Compression) { assert_eq!( match decoder.read_image() { Ok(DecodingResult::U16(image_data)) => image_data, - unexpected => panic!("Descoding RGB failed: {:?}", unexpected), + unexpected => panic!("Descoding RGB failed: {unexpected:?}"), }, image_rgb.reference_data() ); @@ -126,7 +126,7 @@ fn encode_decode_with_compression(compression: Compression) { assert_eq!( match decoder.read_image() { Ok(DecodingResult::U8(image_data)) => image_data, - unexpected => panic!("Decoding grayscale failed: {:?}", unexpected), + unexpected => panic!("Decoding grayscale failed: {unexpected:?}"), }, image_grayscale.reference_data() ); From 46e51cc67e0fd59f325f599eae7fcd309c149a67 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:21:21 +0200 Subject: [PATCH 6/9] Make documentation adhere clippy, more natural --- src/decoder/stream.rs | 24 +++++++++++------------- src/tags.rs | 28 ++++++++++++++++++---------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index 242128f..c2ff0f9 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -135,20 +135,17 @@ impl EndianReader { } } -/// -/// # READERS -/// - -/// -/// ## Deflate Reader -/// +// +// # READERS +// +/// Type alias for the deflate Reader #[cfg(feature = "deflate")] pub type DeflateReader = flate2::read::ZlibDecoder; -/// -/// ## LZW Reader -/// +// +// ## LZW Reader +// /// Reader that decompresses LZW streams #[cfg(feature = "lzw")] @@ -207,10 +204,11 @@ impl Read for LZWReader { } } -/// -/// ## PackBits Reader -/// +// +// ## PackBits Reader +// +/// Internal state machine for the PackBitsReader. enum PackBitsReaderState { Header, Literal, diff --git a/src/tags.rs b/src/tags.rs index 9a9f9f3..05d3f07 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -2,7 +2,7 @@ macro_rules! tags { { // Permit arbitrary meta items, which include documentation. $( #[$enum_attr:meta] )* - $vis:vis enum $name:ident($ty:tt) $(unknown($unknown_doc:literal))* { + $vis:vis enum $name:ident($ty:tt) $(unknown(#[$unknown_meta:meta] $unknown_doc:ident))* { // Each of the `Name = Val,` permitting documentation. $($(#[$ident_attr:meta])* $tag:ident = $val:expr,)* } @@ -13,7 +13,7 @@ macro_rules! tags { pub enum $name { $($(#[$ident_attr])* $tag,)* $( - #[doc = $unknown_doc] + #[$unknown_meta] Unknown($ty), )* } @@ -31,7 +31,7 @@ macro_rules! tags { fn __to_inner_type(&self) -> $ty { match *self { $( $name::$tag => $val, )* - $( $name::Unknown(n) => { $unknown_doc; n }, )* + $( $name::Unknown($unknown_doc) => { $unknown_doc }, )* } } } @@ -39,7 +39,7 @@ macro_rules! tags { tags!($name, $ty, $($unknown_doc)*); }; // For u16 tags, provide direct inherent primitive conversion methods. - ($name:tt, u16, $($unknown_doc:literal)*) => { + ($name:tt, u16, $($unknown_doc:ident)*) => { impl $name { #[inline(always)] pub fn from_u16(val: u16) -> Option { @@ -48,9 +48,8 @@ macro_rules! tags { $( #[inline(always)] - pub fn from_u16_exhaustive(val: u16) -> Self { - $unknown_doc; - Self::__from_inner_type(val).unwrap_or_else(|_| $name::Unknown(val)) + pub fn from_u16_exhaustive($unknown_doc: u16) -> Self { + Self::__from_inner_type($unknown_doc).unwrap_or_else(|_| $name::Unknown($unknown_doc)) } )* @@ -68,7 +67,10 @@ macro_rules! tags { // Note: These tags appear in the order they are mentioned in the TIFF reference tags! { /// TIFF tags -pub enum Tag(u16) unknown("A private or extension tag") { +pub enum Tag(u16) unknown( + /// A private or extension tag + unknown +) { // Baseline tags: Artist = 315, // grayscale images PhotometricInterpretation 1 or 3 @@ -192,7 +194,10 @@ pub enum Type(u16) { tags! { /// See [TIFF compression tags](https://www.awaresystems.be/imaging/tiff/tifftags/compression.html) /// for reference. -pub enum CompressionMethod(u16) unknown("A custom compression method") { +pub enum CompressionMethod(u16) unknown( + /// A custom compression method + unknown +) { None = 1, Huffman = 2, Fax3 = 3, @@ -254,7 +259,10 @@ pub enum ResolutionUnit(u16) { } tags! { -pub enum SampleFormat(u16) unknown("An unknown extension sample format") { +pub enum SampleFormat(u16) unknown( + /// An unknown extension sample format + unknown +) { Uint = 1, Int = 2, IEEEFP = 3, From a3c81e9830745cdca4335029d5e3a9fd7f1b3653 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:25:21 +0200 Subject: [PATCH 7/9] Add Directory::is_empty for completeness --- src/directory.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/directory.rs b/src/directory.rs index 4a33d92..6a2d0ff 100644 --- a/src/directory.rs +++ b/src/directory.rs @@ -27,6 +27,8 @@ pub struct Directory { } impl Directory { + /// Create a directory in an initial state without entries. Note that an empty directory can + /// not be encoded in a file, it must contain at least one entry. pub fn empty() -> Self { Directory { entries: BTreeMap::new(), @@ -72,6 +74,12 @@ impl Directory { self.entries.len() } + /// Check if there are any entries in this directory. Note that an empty directory can not be + /// encoded in the file, it must contain at least one entry. + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } + /// Get the pointer to the next IFD, if it was defined. pub fn next(&self) -> Option { self.next_ifd.map(|n| IfdPointer(n.get())) From d07da5a6cb937e36806a0053413af6fd24bbc06e Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:28:30 +0200 Subject: [PATCH 8/9] Resolve clippy lints in tests --- tests/decode_images.rs | 10 +++++----- tests/encode_images.rs | 2 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/decode_images.rs b/tests/decode_images.rs index a459c86..b53fa82 100644 --- a/tests/decode_images.rs +++ b/tests/decode_images.rs @@ -45,12 +45,12 @@ fn test_image_color_type_unsupported(file: &str, expected_type: ColorType) { let img_file = File::open(path).expect("Cannot find test image!"); let mut decoder = Decoder::new(img_file).expect("Cannot create decoder"); assert_eq!(decoder.colortype().unwrap(), expected_type); - assert!(match decoder.read_image() { + assert!(matches!( + decoder.read_image(), Err(tiff::TiffError::UnsupportedError( tiff::TiffUnsupportedError::UnsupportedColorType(_), - )) => true, - _ => false, - }); + )) + )); } #[test] @@ -172,7 +172,7 @@ fn test_string_tags() { &s, "GraphicsMagick 1.2 unreleased Q16 http://www.GraphicsMagick.org/" ), - _ => assert!(false), + other => unreachable!("Expected Ascii tag from file, got {other:?}"), }; } } diff --git a/tests/encode_images.rs b/tests/encode_images.rs index c183543..ea83b25 100644 --- a/tests/encode_images.rs +++ b/tests/encode_images.rs @@ -652,8 +652,6 @@ fn test_auxiliary_directory() { encoder.write_tag(Tag::ExifDirectory, exif.offset).unwrap(); } - drop(tiff); - data.set_position(0); let mut decoder = Decoder::new(&mut data).unwrap(); From b2bcbbaf2312c9bf4cb7483a720787b28309b2c3 Mon Sep 17 00:00:00 2001 From: "A. Molzer" <5550310+197g@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:28:40 +0200 Subject: [PATCH 9/9] Enable clippy for the merge workflow --- .github/workflows/rust.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 211a546..7e087ca 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -45,6 +45,10 @@ jobs: - name: Cargo check run: cargo check --features "${{ matrix.feature }}" --no-default-features + - name: Cargo clippy + if: ${{ matrix.feature != '' }} + run: cargo clippy --no-deps --all-features --all-targets -- -D warnings + other_archs: # github actions does not support 32-bit or big endian systems directly, but # it does support QEMU. so we install qemu, then build and run the tests on