font_collection_num_fonts_overflow test fails on 32-bit architectures

[jamessan]

---- font_collection_num_fonts_overflow stdout ----
thread 'font_collection_num_fonts_overflow' panicked at 'attempt to multiply with overflow', src/parser.rs:768:19
stack backtrace:
   0: rust_begin_unwind
             at /usr/src/rustc-1.66.0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /usr/src/rustc-1.66.0/library/core/src/panicking.rs:65:14
   2: core::panicking::panic
             at /usr/src/rustc-1.66.0/library/core/src/panicking.rs:115:5
   3: ttf_parser::parser::Stream::read_array32
             at ./src/parser.rs:768:19
   4: ttf_parser::RawFace::parse
             at ./src/lib.rs:669:27
   5: ttf_parser::Face::parse
             at ./src/lib.rs:926:24
   6: tables::font_collection_num_fonts_overflow
             at ./tests/tables/main.rs:136:9
   7: tables::font_collection_num_fonts_overflow::{{closure}}
             at ./tests/tables/main.rs:125:1
   8: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.66.0/library/core/src/ops/function.rs:251:5
   9: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.66.0/library/core/src/ops/function.rs:251:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

[RazrFalcon]

Can confirm.

[plugwash]

The issue is that the code takes a 32-bit number, converts it to a usize and then multiplies it by SIZE. Afaict SIZE is always a small number so on 64-bit systems this can never overflow. However on 32-bit systems it can and does overflow.

The change below makes the test pass, but I'd like some feedback whether this is the correct approach before I go and add it as a distribution patch.

     pub fn read_array32<T: FromData>(&mut self, count: u32) -> Option<LazyArray
-        let len = usize::num_from(count) * T::SIZE;
-        self.read_bytes(len).map(LazyArray32::new)
+        if let Some(len) = usize::num_from(count).checked_mul(T::SIZE) {
+            self.read_bytes(len).map(LazyArray32::new)
+        } else {
+            None
+        }
     }

[RazrFalcon]

@plugwash yes, I know that this is because usize is 32bit here, but I want a global solution and not a local one. Will think about it.

[NoisyCoil]

I think this issue can be closed at this point? I recently upgraded to v0.24.1 in Debian and the refactored tests passed with no patches.