Put HashMap logic into symbol creation

Author: OndrikB

objdiff-core/src/obj/read.rs

@@ -1,4 +1,10 @@
-use std::{collections::{HashMap, HashSet}, fs, io::Cursor, mem::size_of, path::Path};
+use std::{
+    collections::{HashMap, HashSet},
+    fs,
+    io::Cursor,
+    mem::size_of,
+    path::Path,
+};
 
 use anyhow::{anyhow, bail, ensure, Context, Result};
 use filetime::FileTime;
@@ -136,6 +142,7 @@ fn symbols_by_section(
     obj_file: &File<'_>,
     section: &ObjSection,
     split_meta: Option<&SplitMeta>,
+    name_counts: &mut HashMap<String, u32>,
 ) -> Result<Vec<ObjSymbol>> {
     let mut result = Vec::<ObjSymbol>::new();
     for symbol in obj_file.symbols() {
@@ -168,8 +175,14 @@ fn symbols_by_section(
     }
     if result.is_empty() {
         // Dummy symbol for empty sections
+        *name_counts.entry(section.name.clone()).or_insert(0) += 1;
+        let current_count: u32 = *name_counts.get(&section.name).unwrap();
         result.push(ObjSymbol {
-            name: format!("[{}]", section.name),
+            name: if current_count > 1 {
+                format!("[{} ({})]", section.name, current_count)
+            } else {
+                format!("[{}]", section.name)
+            },
             demangled_name: None,
             address: 0,
             section_address: 0,
@@ -621,33 +634,18 @@ pub fn parse(data: &[u8], config: &DiffObjConfig) -> Result<ObjInfo> {
     let arch = new_arch(&obj_file)?;
     let split_meta = split_meta(&obj_file)?;
     let mut sections = filter_sections(&obj_file, split_meta.as_ref())?;
+    let mut name_counts: HashMap<String, u32> = HashMap::new();
     for section in &mut sections {
-        section.symbols =
-            symbols_by_section(arch.as_ref(), &obj_file, section, split_meta.as_ref())?;
+        section.symbols = symbols_by_section(
+            arch.as_ref(),
+            &obj_file,
+            section,
+            split_meta.as_ref(),
+            &mut name_counts,
+        )?;
         section.relocations =
             relocations_by_section(arch.as_ref(), &obj_file, section, split_meta.as_ref())?;
     }
-    // The dummy symbols all have the same name, which means that only the first one can be
-    // compared
-    let all_names = sections
-        .iter()
-        .flat_map(|section| section.symbols.iter().map(|symbol| symbol.name.clone()));
-    let mut name_counts: HashMap<String, usize> = HashMap::new();
-    for name in all_names {
-        name_counts.entry(name).and_modify(|i| *i += 1).or_insert(1);
-    }
-    // Reversing the iterator here means the symbol sections will be given in the expected order
-    // since they're assigned from the count down to prevent
-    // having to use another hashmap which counts up
-    // TODO what about reverse_fn_order?
-    for section in sections.iter_mut().rev() {
-        for symbol in section.symbols.iter_mut().rev() {
-            if name_counts[&symbol.name] > 1 {
-                name_counts.entry(symbol.name.clone()).and_modify(|i| *i -= 1);
-                symbol.name = format!("{} ({})", &symbol.name, name_counts[&symbol.name]);
-            }
-        }
-    }
     if config.combine_data_sections {
         combine_data_sections(&mut sections)?;
     }