Merge pull request #116 from willglynn/fix/clippy-lints

Fix lints, idioms, tests, and remove dead code where applicable.

examples/pdb2hpp.rs

@@ -322,7 +322,7 @@ struct BaseClass {
 struct Field<'p> {
     type_name: String,
     name: pdb::RawString<'p>,
-    offset: u16,
+    offset: u64,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -611,22 +611,12 @@ fn write_class(filename: &str, class_name: &str) -> pdb::Result<()> {
     }
 
     // add all the needed types iteratively until we're done
-    loop {
-        // get the last element in needed_types without holding an immutable borrow
-        let last = match needed_types.iter().next_back() {
-            Some(n) => Some(*n),
-            None => None,
-        };
-
-        if let Some(type_index) = last {
-            // remove it
-            needed_types.remove(&type_index);
-
-            // add the type
-            data.add(&type_finder, type_index, &mut needed_types)?;
-        } else {
-            break;
-        }
+    while let Some(type_index) = needed_types.iter().next_back().copied() {
+        // remove it
+        needed_types.remove(&type_index);
+
+        // add the type
+        data.add(&type_finder, type_index, &mut needed_types)?;
     }
 
     if data.classes.is_empty() {
@@ -652,7 +642,7 @@ fn main() {
 
     let matches = match opts.parse(&args[1..]) {
         Ok(m) => m,
-        Err(f) => panic!(f.to_string()),
+        Err(f) => panic!("{}", f.to_string()),
     };
 
     let (filename, class_name) = if matches.free.len() == 2 {
@@ -662,7 +652,7 @@ fn main() {
         return;
     };
 
-    match write_class(&filename, &class_name) {
+    match write_class(filename, class_name) {
         Ok(_) => (),
         Err(e) => eprintln!("error dumping PDB: {}", e),
     }

examples/pdb_framedata.rs

@@ -39,7 +39,7 @@ fn dump_framedata(filename: &str) -> pdb::Result<()> {
             if data.has_cpp_eh { 'Y' } else { 'N' },
             if data.is_function_start { 'Y' } else { 'N' },
             if data.uses_base_pointer { 'Y' } else { 'N' },
-            data.ty.to_string(),
+            data.ty,
             program_string,
         );
     }
@@ -55,7 +55,7 @@ fn main() {
     opts.optflag("h", "help", "print this help menu");
     let matches = match opts.parse(&args[1..]) {
         Ok(m) => m,
-        Err(f) => panic!(f.to_string()),
+        Err(f) => panic!("{}", f.to_string()),
     };
 
     let filename = if matches.free.len() == 1 {
@@ -65,7 +65,7 @@ fn main() {
         return;
     };
 
-    match dump_framedata(&filename) {
+    match dump_framedata(filename) {
         Ok(_) => (),
         Err(e) => eprintln!("error dumping PDB: {}", e),
     }

examples/pdb_lines.rs

@@ -56,7 +56,7 @@ fn main() {
     opts.optflag("h", "help", "print this help menu");
     let matches = match opts.parse(&args[1..]) {
         Ok(m) => m,
-        Err(f) => panic!(f.to_string()),
+        Err(f) => panic!("{}", f.to_string()),
     };
 
     let filename = if matches.free.len() == 1 {
@@ -67,7 +67,7 @@ fn main() {
         return;
     };
 
-    match dump_pdb(&filename) {
+    match dump_pdb(filename) {
         Ok(_) => {}
         Err(e) => {
             writeln!(&mut std::io::stderr(), "error dumping PDB: {}", e).expect("stderr write");

examples/pdb_symbols.rs

@@ -80,7 +80,7 @@ fn main() {
     opts.optflag("h", "help", "print this help menu");
     let matches = match opts.parse(&args[1..]) {
         Ok(m) => m,
-        Err(f) => panic!(f.to_string()),
+        Err(f) => panic!("{}", f.to_string()),
     };
 
     let filename = if matches.free.len() == 1 {
@@ -90,7 +90,7 @@ fn main() {
         return;
     };
 
-    match dump_pdb(&filename) {
+    match dump_pdb(filename) {
         Ok(_) => (),
         Err(e) => eprintln!("error dumping PDB: {}", e),
     }

src/common.rs

@@ -681,7 +681,7 @@ impl_convert!(Register, u16);
 impl_pread!(Register);
 
 /// Provides little-endian access to a &[u8].
-#[derive(Debug, Clone)]
+#[derive(Debug, Default, Clone)]
 pub(crate) struct ParseBuffer<'b>(&'b [u8], usize);
 
 macro_rules! def_parse {
@@ -690,7 +690,7 @@ macro_rules! def_parse {
           #[inline]
           #[allow(unused)]
           pub fn $n(&mut self) -> Result<$t> {
-              Ok(self.parse()?)
+              self.parse()
           })*
     }
 }
@@ -821,12 +821,6 @@ impl<'b> ParseBuffer<'b> {
     }
 }
 
-impl Default for ParseBuffer<'_> {
-    fn default() -> Self {
-        ParseBuffer(&[], 0)
-    }
-}
-
 impl<'b> From<&'b [u8]> for ParseBuffer<'b> {
     fn from(buf: &'b [u8]) -> Self {
         ParseBuffer(buf, 0)

src/dbi.rs

@@ -142,6 +142,7 @@ impl From<u32> for HeaderVersion {
 /// Reference:
 /// https://github.com/Microsoft/microsoft-pdb/blob/082c5290e5aff028ae84e43affa8be717aa7af73/PDB/dbi/dbi.h#L124
 #[derive(Debug, Copy, Clone)]
+#[allow(dead_code)] // reason = "unused fields added for completeness"
 pub(crate) struct DBIHeader {
     pub signature: u32,
     pub version: HeaderVersion,
@@ -373,7 +374,7 @@ impl From<u16> for MachineType {
 
 /// Information about a module's contribution to a section.
 /// `struct SC` in Microsoft's code:
-/// https://github.com/Microsoft/microsoft-pdb/blob/082c5290e5aff028ae84e43affa8be717aa7af73/PDB/include/dbicommon.h#L42
+/// <https://github.com/Microsoft/microsoft-pdb/blob/082c5290e5aff028ae84e43affa8be717aa7af73/PDB/include/dbicommon.h#L42>
 #[derive(Debug, Copy, Clone)]
 pub struct DBISectionContribution {
     /// Start offset of the section.
@@ -418,6 +419,7 @@ impl DBISectionContribution {
 /// the Microsoft PDB source:
 /// https://github.com/Microsoft/microsoft-pdb/blob/082c5290e5aff028ae84e43affa8be717aa7af73/PDB/dbi/dbi.h#L1197
 #[derive(Debug, Copy, Clone)]
+#[allow(dead_code)] // reason = "unused fields added for completeness"
 pub(crate) struct DBIModuleInfo {
     /// Currently open module.
     pub opened: u32,
@@ -583,6 +585,7 @@ impl<'c> FallibleIterator for DBISectionContributionIter<'c> {
 
 /// A `DbgDataHdr`, which contains a series of (optional) MSF stream numbers.
 #[derive(Debug, Copy, Clone)]
+#[allow(dead_code)] // reason = "unused fields added for completeness"
 pub(crate) struct DBIExtraStreams {
     // The struct itself is defined at:
     //    https://github.com/Microsoft/microsoft-pdb/blob/082c5290e5aff028ae84e43affa8be717aa7af73/PDB/dbi/dbi.h#L250-L274

src/modi/c13.rs

@@ -231,6 +231,7 @@ struct DebugLinesHeader {
     /// See LineFlags enumeration.
     flags: u16,
     /// Code size of this line contribution.
+    #[allow(dead_code)] // reason = "unused until TODO in LineIterator is resolved"
     code_size: u32,
 }
 
@@ -330,8 +331,10 @@ struct LineNumberEntry {
 #[derive(Clone, Debug)]
 struct LineMarkerEntry {
     /// Delta offset to the start of this line contribution (debug lines subsection).
+    #[allow(dead_code)] // reason = "unused until TODO in LineIterator is resolved"
     pub offset: u32,
     /// The marker kind, hinting a debugger how to deal with code at this offset.
+    #[allow(dead_code)] // reason = "debugger instructions are not exposed"
     pub kind: LineMarkerKind,
 }
 
@@ -437,7 +440,6 @@ impl<'t> TryFromCtx<'t, Endian> for ColumnNumberEntry {
 
 #[derive(Clone, Debug, Default)]
 struct DebugColumnsIterator<'a> {
-    block: DebugLinesBlockHeader,
     buf: ParseBuffer<'a>,
 }
 
@@ -527,8 +529,7 @@ impl<'a> DebugLinesBlock<'a> {
 
     fn columns(&self) -> DebugColumnsIterator<'a> {
         DebugColumnsIterator {
-            block: self.header,
-            buf: ParseBuffer::from(self.line_data),
+            buf: ParseBuffer::from(self.column_data),
         }
     }
 }

src/msf/mod.rs

@@ -315,7 +315,7 @@ mod big {
         }
     }
 
-    impl<'s, S: Source<'s>> MSF<'s, S> for BigMSF<'s, S> {
+    impl<'s, S: Source<'s>> Msf<'s, S> for BigMSF<'s, S> {
         fn get(&mut self, stream_number: u32, limit: Option<usize>) -> Result<Stream<'s>> {
             // look up the stream
             let mut page_list = self.look_up_stream(stream_number)?;
@@ -370,7 +370,7 @@ impl Deref for Stream<'_> {
 }
 
 /// Provides access to a "multi-stream file", which is the container format used by PDBs.
-pub trait MSF<'s, S>: fmt::Debug {
+pub trait Msf<'s, S>: fmt::Debug {
     /// Accesses a stream by stream number, optionally restricted by a byte limit.
     fn get(&mut self, stream_number: u32, limit: Option<usize>) -> Result<Stream<'s>>;
 }
@@ -379,7 +379,7 @@ fn header_matches(actual: &[u8], expected: &[u8]) -> bool {
     actual.len() >= expected.len() && &actual[0..expected.len()] == expected
 }
 
-pub fn open_msf<'s, S: Source<'s> + 's>(mut source: S) -> Result<Box<dyn MSF<'s, S> + 's>> {
+pub fn open_msf<'s, S: Source<'s> + 's>(mut source: S) -> Result<Box<dyn Msf<'s, S> + 's>> {
     // map the header
     let mut header_location = PageList::new(4096);
     header_location.push(0);

src/pdb.rs

@@ -9,7 +9,7 @@ use crate::common::*;
 use crate::dbi::{DBIExtraStreams, DBIHeader, DebugInformation, Module};
 use crate::framedata::FrameTable;
 use crate::modi::ModuleInfo;
-use crate::msf::{self, Stream, MSF};
+use crate::msf::{self, Msf, Stream};
 use crate::omap::{AddressMap, OMAPTable};
 use crate::pdbi::PDBInformation;
 use crate::pe::ImageSectionHeader;
@@ -34,7 +34,7 @@ const IPI_STREAM: u32 = 4;
 #[derive(Debug)]
 pub struct PDB<'s, S> {
     /// `msf` provides access to the underlying data streams
-    msf: Box<dyn MSF<'s, S> + 's>,
+    msf: Box<dyn Msf<'s, S> + 's>,
 
     /// Memoize the `dbi::Header`, since it contains stream numbers we sometimes need
     dbi_header: Option<DBIHeader>,

src/pdbi.rs

@@ -117,32 +117,31 @@ impl<'s> PDBInformation<'s> {
         // [4]: https://github.com/Microsoft/microsoft-pdb/blob/082c5290e5aff028ae84e43affa8be717aa7af73/PDB/include/array.h#L209
 
         let mut names = vec![];
-        let buf = {
-            let mut buf = self.stream.parse_buffer();
-            // Seek forward to the name map.
-            buf.take(self.names_offset + self.names_size)?;
-            let count = buf.parse_u32()?;
-            // We don't actually use most of these.
-            let _entries_size = buf.parse_u32()?;
-            let ok_words = buf.parse_u32()?;
-            let _ok_bits = buf.take(ok_words as usize * mem::size_of::<u32>())?;
-            let deleted_words = buf.parse_u32()?;
-            let _deleted_bits = buf.take(deleted_words as usize * mem::size_of::<u32>())?;
-
-            // Skip over the header here.
-            let mut names_reader = self.stream.parse_buffer();
-            names_reader.take(self.names_offset)?;
-            // And take just the name data.
-            let names_buf = names_reader.take(self.names_size)?;
-            for _ in 0..count {
-                let name_offset = buf.parse_u32()? as usize;
-                let stream_id = StreamIndex(buf.parse_u32()? as u16);
-                let name = ParseBuffer::from(&names_buf[name_offset..]).parse_cstring()?;
-                names.push(StreamName { name, stream_id });
-            }
-            names_reader
-        };
-        Ok(StreamNames { buf, names })
+        let mut buf = self.stream.parse_buffer();
+
+        // Seek forward to the name map.
+        buf.take(self.names_offset + self.names_size)?;
+        let count = buf.parse_u32()?;
+        // We don't actually use most of these.
+        let _entries_size = buf.parse_u32()?;
+        let ok_words = buf.parse_u32()?;
+        let _ok_bits = buf.take(ok_words as usize * mem::size_of::<u32>())?;
+        let deleted_words = buf.parse_u32()?;
+        let _deleted_bits = buf.take(deleted_words as usize * mem::size_of::<u32>())?;
+
+        // Skip over the header here.
+        let mut names_reader = self.stream.parse_buffer();
+        names_reader.take(self.names_offset)?;
+        // And take just the name data.
+        let names_buf = names_reader.take(self.names_size)?;
+        for _ in 0..count {
+            let name_offset = buf.parse_u32()? as usize;
+            let stream_id = StreamIndex(buf.parse_u32()? as u16);
+            let name = ParseBuffer::from(&names_buf[name_offset..]).parse_cstring()?;
+            names.push(StreamName { name, stream_id });
+        }
+
+        Ok(StreamNames { names })
     }
 }
 
@@ -161,7 +160,6 @@ pub struct StreamName<'n> {
 /// objects.
 #[derive(Debug)]
 pub struct StreamNames<'s> {
-    buf: ParseBuffer<'s>,
     /// The list of streams and their names.
     names: Vec<StreamName<'s>>,
 }

src/pe.rs

@@ -92,7 +92,7 @@ impl ImageSectionHeader {
             .name
             .iter()
             .position(|ch| *ch == 0)
-            .unwrap_or_else(|| self.name.len());
+            .unwrap_or(self.name.len());
 
         // The spec guarantees that the name is a proper UTF-8 string.
         // TODO: Look up long names from the string table.

src/strings.rs

@@ -83,6 +83,7 @@ impl StringTableHeader {
 #[derive(Debug)]
 pub struct StringTable<'s> {
     header: StringTableHeader,
+    #[allow(dead_code)] // reason = "reverse-lookups through hash table not implemented"
     hash_version: StringTableHashVersion,
     stream: Stream<'s>,
 }

src/symbol/mod.rs

@@ -61,7 +61,7 @@ impl<'t> Symbol<'t> {
     /// Parse the symbol into the `SymbolData` it contains.
     #[inline]
     pub fn parse(&self) -> Result<SymbolData<'t>> {
-        Ok(self.raw_bytes().pread_with(0, ())?)
+        self.raw_bytes().pread_with(0, ())
     }
 
     /// Returns whether this symbol starts a scope.