Merge pull request #201 from dodomorandi/remove-unsafe

Remove unsafe

src/cfb.rs

@@ -83,15 +83,15 @@ impl Cfb {
  debug!("load difat");
  let mut sector_id = h.difat_start;
  while sector_id < RESERVED_SECTORS {
- difat.extend_from_slice(to_u32(sectors.get(sector_id, reader)?));
+ difat.extend(to_u32(sectors.get(sector_id, reader)?));
  sector_id = difat.pop().unwrap(); //TODO: check if in infinite loop
  }
 
  // load the FATs
  debug!("load fat");
  let mut fats = Vec::with_capacity(h.fat_len);
  for id in difat.into_iter().filter(|id| *id != FREESECT) {
- fats.extend_from_slice(to_u32(sectors.get(id, reader)?));
+ fats.extend(to_u32(sectors.get(id, reader)?));
  }
 
  // get the list of directory sectors
@@ -116,7 +116,7 @@ impl Cfb {
  reader,
  h.mini_fat_len * h.sector_size,
  )?;
- let minifat = to_u32(&minifat).to_vec();
+ let minifat = to_u32(&minifat).collect();
  Ok(Cfb {
  directories: dirs,
  sectors,
@@ -167,7 +167,7 @@ impl Header {
  f.read_exact(&mut buf).map_err(CfbError::Io)?;
 
  // check ole signature
- if read_slice::<u64>(buf.as_ref()) != 0xE11A_B1A1_E011_CFD0 {
+ if read_slice_u64(buf.as_ref()).next() != Some(0xE11A_B1A1_E011_CFD0) {
  return Err(CfbError::Ole);
  }
 
@@ -208,7 +208,7 @@ impl Header {
  let difat_len = read_usize(&buf[62..76]);
 
  let mut difat = Vec::with_capacity(difat_len);
- difat.extend_from_slice(to_u32(&buf[76..512]));
+ difat.extend(to_u32(&buf[76..512]));
 
  Ok((
  Header {
@@ -245,9 +245,7 @@ impl Sectors {
  let end = start + self.size;
  if end > self.data.len() {
  let mut len = self.data.len();
- unsafe {
- self.data.set_len(end);
- }
+ self.data.resize(end, 0);
  // read_exact or stop if EOF
  while len < end {
  let read = r.read(&mut self.data[len..end]).map_err(CfbError::Io)?;
@@ -293,15 +291,20 @@ struct Directory {
 
 impl Directory {
  fn from_slice(buf: &[u8], sector_size: usize) -> Directory {
+ use std::convert::TryFrom;
+
  let mut name = UTF_16LE.decode(&buf[..64]).0.into_owned();
  if let Some(l) = name.as_bytes().iter().position(|b| *b == 0) {
  name.truncate(l);
  }
  let start = read_u32(&buf[116..120]);
  let len = if sector_size == 512 {
- read_slice::<u32>(&buf[120..124]) as usize
+ read_slice_u32(&buf[120..124]).next().unwrap() as usize
  } else {
- read_slice::<u64>(&buf[120..128]) as usize
+ read_slice_u64(&buf[120..128])
+ .next()
+ .and_then(|x| usize::try_from(x).ok())
+ .unwrap()
  };
 
  Directory { start, len, name }

src/utils.rs

@@ -12,21 +12,50 @@ macro_rules! from_err {
  };
 }
 
-/// Converts a &[u8] into a &[u32]
-pub fn to_u32(s: &[u8]) -> &[u32] {
+/// Converts a &[u8] into an iterator of `u32`s
+pub fn to_u32(s: &[u8]) -> impl ExactSizeIterator<Item = u32> + '_ {
  assert_eq!(s.len() % 4, 0);
- unsafe { std::slice::from_raw_parts(s as *const [u8] as *const u32, s.len() / 4) }
+ s.chunks_exact(4)
+ .map(|data| u32::from_ne_bytes([data[0], data[1], data[2], data[3]]))
 }
-pub fn read_slice<T>(s: &[u8]) -> T {
- unsafe { std::ptr::read(&s[..std::mem::size_of::<T>()] as *const [u8] as *const T) }
+
+pub(crate) fn read_slice_u16(s: &[u8]) -> impl ExactSizeIterator<Item = u16> + '_ {
+ s.chunks_exact(2)
+ .map(|chunk| u16::from_ne_bytes([chunk[0], chunk[1]]))
+}
+
+pub(crate) fn read_slice_i32(s: &[u8]) -> impl ExactSizeIterator<Item = i32> + '_ {
+ s.chunks_exact(4)
+ .map(|chunk| i32::from_ne_bytes([chunk[0], chunk[1], chunk[2], chunk[3]]))
+}
+
+pub(crate) fn read_slice_u32(s: &[u8]) -> impl ExactSizeIterator<Item = u32> + '_ {
+ s.chunks_exact(4)
+ .map(|chunk| u32::from_ne_bytes([chunk[0], chunk[1], chunk[2], chunk[3]]))
+}
+
+pub(crate) fn read_slice_u64(s: &[u8]) -> impl ExactSizeIterator<Item = u64> + '_ {
+ s.chunks_exact(8).map(|chunk| {
+ u64::from_ne_bytes([
+ chunk[0], chunk[1], chunk[2], chunk[3], chunk[4], chunk[5], chunk[6], chunk[7],
+ ])
+ })
+}
+
+pub(crate) fn read_slice_f64(s: &[u8]) -> impl ExactSizeIterator<Item = f64> + '_ {
+ s.chunks_exact(8).map(|chunk| {
+ f64::from_ne_bytes([
+ chunk[0], chunk[1], chunk[2], chunk[3], chunk[4], chunk[5], chunk[6], chunk[7],
+ ])
+ })
 }
 
 pub fn read_u32(s: &[u8]) -> u32 {
- read_slice(s)
+ read_slice_u32(s).next().unwrap()
 }
 
 pub fn read_u16(s: &[u8]) -> u16 {
- read_slice(s)
+ read_slice_u16(s).next().unwrap()
 }
 
 pub fn read_usize(s: &[u8]) -> usize {
@@ -1030,3 +1059,17 @@ pub const FTAB_ARGC: [u8; FTAB_LEN] = [
  3, // "AVERAGEIF",
  129, // "AVERAGEIFS"
 ];
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn sound_to_u32() {
+ let data = b"ABCDEFGH";
+ assert_eq!(
+ to_u32(data).collect::<Vec<_>>(),
+ [u32::from_ne_bytes(*b"ABCD"), u32::from_ne_bytes(*b"EFGH")]
+ );
+ }
+}

src/xls.rs

@@ -1,13 +1,14 @@
 use std::borrow::Cow;
 use std::cmp::min;
 use std::collections::HashMap;
+use std::convert::TryFrom;
 use std::io::{Read, Seek, SeekFrom};
 use std::marker::PhantomData;
 
 use log::debug;
 
 use crate::cfb::{Cfb, XlsEncoding};
-use crate::utils::{push_column, read_slice, read_u16, read_u32};
+use crate::utils::{push_column, read_slice_f64, read_slice_i32, read_u16, read_u32};
 use crate::vba::VbaProject;
 use crate::{Cell, CellErrorType, DataType, Metadata, Range, Reader};
 
@@ -317,7 +318,7 @@ fn parse_number(r: &[u8]) -> Result<Cell<DataType>, XlsError> {
  }
  let row = read_u16(r) as u32;
  let col = read_u16(&r[2..]) as u32;
- let v = read_slice::<f64>(&r[6..]);
+ let v = read_slice_f64(&r[6..]).next().unwrap();
  Ok(Cell::new((row, col), DataType::Float(v)))
 }
 
@@ -411,14 +412,14 @@ fn rk_num(rk: &[u8]) -> DataType {
  v[4..].copy_from_slice(rk);
  v[0] &= 0xFC;
  if is_int {
- let v = (read_slice::<i32>(&v[4..]) >> 2) as i64;
+ let v = (read_slice_i32(&v[4..]).next().unwrap() >> 2) as i64;
  if d100 && v % 100 != 0 {
  DataType::Float(v as f64 / 100.0)
  } else {
  DataType::Int(if d100 { v / 100 } else { v })
  }
  } else {
- let v = read_slice(&v);
+ let v = read_slice_f64(&v).next().unwrap();
  DataType::Float(if d100 { v / 100.0 } else { v })
  }
 }
@@ -507,7 +508,10 @@ fn parse_sst(r: &mut Record<'_>, encoding: &mut XlsEncoding) -> Result<Vec<Strin
  found: r.data.len(),
  });
  }
- let len = read_slice::<i32>(&r.data[4..]) as usize;
+ let len = read_slice_i32(&r.data[4..])
+ .next()
+ .and_then(|x| usize::try_from(x).ok())
+ .unwrap();
  let mut sst = Vec::with_capacity(len);
  r.data = &r.data[8..];
  for _ in 0..len {
@@ -546,7 +550,10 @@ fn read_rich_extended_string(
  0
  };
  if ext_st != 0 {
- unused_len += read_slice::<i32>(r.data) as usize;
+ unused_len += read_slice_i32(r.data)
+ .next()
+ .and_then(|x| usize::try_from(x).ok())
+ .unwrap();
  r.data = &r.data[4..];
  };
 
@@ -897,7 +904,7 @@ fn parse_formula(
  }
  0x1F => {
  stack.push(formula.len());
- formula.push_str(&format!("{}", read_slice::<f64>(rgce)));
+ formula.push_str(&format!("{}", read_slice_f64(rgce).next().unwrap()));
  rgce = &rgce[8..];
  }
  0x20 | 0x40 | 0x60 => {

src/xlsb.rs

@@ -12,7 +12,7 @@ use quick_xml::Reader as XmlReader;
 use zip::read::{ZipArchive, ZipFile};
 use zip::result::ZipError;
 
-use crate::utils::{push_column, read_slice, read_u16, read_u32, read_usize};
+use crate::utils::{push_column, read_slice_f64, read_slice_i32, read_u16, read_u32, read_usize};
 use crate::vba::VbaProject;
 use crate::{Cell, CellErrorType, DataType, Metadata, Range, Reader};
 
@@ -255,7 +255,7 @@ impl<RS: Read + Seek> Xlsb<RS> {
  let extern_sheets = buf[4..]
  .chunks(12)
  .map(|xti| {
- match read_slice::<i32>(&xti[4..8]) {
+ match read_slice_i32(&xti[4..8]).next().unwrap() {
  -2 => "#ThisWorkbook",
  -1 => "#InvalidWorkSheet",
  p if p >= 0 && (p as usize) < sheets.len() => &sheets[p as usize].0,
@@ -337,7 +337,7 @@ impl<RS: Read + Seek> Xlsb<RS> {
  let is_int = (buf[8] & 2) != 0;
  buf[8] &= 0xFC;
  if is_int {
- let v = (read_slice::<i32>(&buf[8..12]) >> 2) as i64;
+ let v = (read_slice_i32(&buf[8..12]).next().unwrap() >> 2) as i64;
  if d100 {
  DataType::Float((v as f64) / 100.0)
  } else {
@@ -346,7 +346,7 @@ impl<RS: Read + Seek> Xlsb<RS> {
  } else {
  let mut v = [0u8; 8];
  v[4..].copy_from_slice(&buf[8..12]);
- let v = read_slice(&v);
+ let v = read_slice_f64(&v).next().unwrap();
  DataType::Float(if d100 { v / 100.0 } else { v })
  }
  }
@@ -366,7 +366,7 @@ impl<RS: Read + Seek> Xlsb<RS> {
  DataType::Error(error)
  }
  0x0004 | 0x000A => DataType::Bool(buf[8] != 0), // BrtCellBool or BrtFmlaBool
- 0x0005 | 0x0009 => DataType::Float(read_slice(&buf[8..16])), // BrtCellReal or BrtFmlaFloat
+ 0x0005 | 0x0009 => DataType::Float(read_slice_f64(&buf[8..16]).next().unwrap()), // BrtCellReal or BrtFmlaFloat
  0x0006 | 0x0008 => DataType::String(wide_str(&buf[8..], &mut 0)?.into_owned()), // BrtCellSt or BrtFmlaString
  0x0007 => {
  // BrtCellIsst
@@ -809,7 +809,7 @@ fn parse_formula(
  }
  0x1F => {
  stack.push(formula.len());
- formula.push_str(&format!("{}", read_slice::<f64>(rgce)));
+ formula.push_str(&format!("{}", read_slice_f64(rgce).next().unwrap()));
  rgce = &rgce[8..];
  }
  0x20 | 0x40 | 0x60 => {