Refactor SymbolTables

src/location.rs

@@ -11,7 +11,7 @@ pub enum Location {
 impl Location {
     pub fn addr(&self, symbol_tables: &SymbolTables) -> u32 {
         match self {
-            Self::Symbol((table, sym)) => symbol_tables[table].symbols[sym].value(),
+            Self::Symbol((table, sym)) => symbol_tables.get(table, sym).unwrap().value(),
             Self::Address(addr) => *addr,
         }
     }

src/main.rs

@@ -12,17 +12,16 @@ mod rom;
 mod storage;
 mod symbol;
 mod symbol_table;
+mod symbol_tables;
 mod term;
 use crate::{
     backplane::Backplane,
     error::Error,
     location::Location,
     m68k::{BusError, M68K},
-    symbol::Symbol,
 };
 use disas::DisassemblyError;
-use elf::gabi::{STT_FILE, STT_SECTION};
-use indexmap::{IndexMap, IndexSet};
+use indexmap::IndexSet;
 use itertools::Itertools;
 use parse_int::parse;
 use reedline_repl_rs::{
@@ -31,17 +30,8 @@ use reedline_repl_rs::{
 };
 use serde::Deserialize;
 use serde_yaml::Mapping;
-use std::{
-    collections::HashMap,
-    convert::TryFrom,
-    error::Error as StdError,
-    fs::{self, File},
-    path::Path,
-    process,
-};
-use symbol_table::SymbolTable;
-
-pub type SymbolTables = IndexMap<String, SymbolTable>;
+use std::{convert::TryFrom, fs, path::Path, process};
+use symbol_tables::SymbolTables;
 
 #[derive(Copy, Clone, Debug)]
 enum PeekFormat {
@@ -156,14 +146,10 @@ fn main() -> Result<(), ReplError> {
             Err(e) => panic!("{}", e),
         };
     }
-    let mut symbol_tables = IndexMap::new();
+    let mut symbol_tables = SymbolTables::new();
     if let Some(initial_tables) = config.symbol_tables {
         for path in initial_tables {
-            let table_name = Path::new(&path).file_name().unwrap().to_str().unwrap();
-            symbol_tables.insert(
-                table_name.to_string(),
-                SymbolTable::new(read_symbol_table(path).unwrap()),
-            );
+            symbol_tables.load_table(path, true).unwrap();
         }
     }
     Repl::<_, Error>::new(EmuState {
@@ -294,14 +280,11 @@ fn main() -> Result<(), ReplError> {
             while !state.cpu.stopped {
                 let stop_addr = args
                     .get_one::<String>("stop_addr")
-                    .map(|s| parse_location_address(s, &state.symbol_tables))
+                    .map(|s| state.symbol_tables.parse_location_address(s))
                     .transpose()?;
                 if stop_addr.map(|a| state.cpu.pc() == a).unwrap_or(false)
-                    | breakpoint_set_at(
-                        state.cpu.pc(),
-                        &state.symbol_tables,
-                        &state.address_breakpoints,
-                    )
+                    || state.symbol_tables.breakpoint_set_at(state.cpu.pc())
+                    || state.address_breakpoints.contains(&state.cpu.pc())
                 {
                     break;
                 }
@@ -342,10 +325,9 @@ fn main() -> Result<(), ReplError> {
             let fmt = PeekFormat::try_from(fmt_str.chars().next().unwrap())?;
             let size = PeekSize::try_from(fmt_str.chars().nth(1).unwrap())?;
             let count = parse::<u32>(args.get_one::<String>("count").map_or("1", String::as_str))?;
-            let addr = parse_location_address(
-                args.get_one::<String>("addr").unwrap(),
-                &state.symbol_tables,
-            )?;
+            let addr = state
+                .symbol_tables
+                .parse_location_address(args.get_one::<String>("addr").unwrap())?;
             let mut data = Vec::new();
             let bus = state.cpu.bus_mut();
             for i in 0..count {
@@ -391,7 +373,7 @@ fn main() -> Result<(), ReplError> {
             let mut addr = args
                 .get_one::<String>("addr")
                 .map_or(Ok(state.cpu.pc()), |s| {
-                    parse_location_address(s, &state.symbol_tables)
+                    state.symbol_tables.parse_location_address(s)
                 })?;
             let count = parse::<u32>(args.get_one::<String>("count").map_or("1", String::as_str))?;
             let mut out = String::new();
@@ -446,51 +428,21 @@ fn main() -> Result<(), ReplError> {
             if let Some(file_path) = args.get_one::<String>("file") {
                 let table_name = Path::new(&file_path).file_name().unwrap().to_str().unwrap();
                 if args.get_flag("delete") {
-                    if state.symbol_tables.shift_remove(table_name).is_some() {
-                        Ok(None)
-                    } else {
-                        Ok(Some("No such symbol table".to_string()))
-                    }
+                    state.symbol_tables.delete(table_name)?;
+                    Ok(None)
                 } else if let Some(&active) = args.get_one::<bool>("active") {
-                    if let Some(table) = state.symbol_tables.get_mut(table_name) {
-                        table.active = active;
-                        Ok(None)
-                    } else {
-                        Ok(Some("No such symbol table".to_string()))
-                    }
+                    state.symbol_tables.set_active(table_name, active)?;
+                    Ok(None)
                 } else {
-                    let symbols = read_symbol_table(file_path)?;
-                    if let Some(table) = state.symbol_tables.get_mut(table_name) {
-                        table.update_symbols(symbols);
-                    } else {
-                        state
-                            .symbol_tables
-                            .insert(table_name.to_string(), SymbolTable::new(symbols));
-                    };
-                    if !args.get_flag("append") {
-                        let table = state.symbol_tables.remove(table_name).unwrap();
-                        state.symbol_tables.clear();
-                        state.symbol_tables.insert(table_name.to_string(), table);
-                    };
+                    state
+                        .symbol_tables
+                        .load_table(file_path, args.get_flag("append"))?;
                     Ok(None)
                 }
+            } else if state.symbol_tables.is_empty() {
+                Ok(Some("No symbols".to_string()))
             } else {
-                let mut out = String::new();
-                for (table_name, table) in state.symbol_tables.iter() {
-                    out += &format!(
-                        "{table_name} ({}): \n",
-                        if table.active { "active" } else { "inactive" }
-                    );
-                    for (name, symbol) in table.symbols.iter() {
-                        out += &format!("{name}: {symbol}\n");
-                    }
-                }
-                out.pop(); // Remove trailing newline
-                if out.is_empty() {
-                    Ok(Some("No symbols".to_string()))
-                } else {
-                    Ok(Some(out))
-                }
+                Ok(Some(format!("{}", state.symbol_tables.symbol_displayer())))
             }
         },
     )
@@ -506,7 +458,10 @@ fn main() -> Result<(), ReplError> {
             let location = args.get_one::<String>("location").unwrap();
             Ok(Some(format!(
                 "{}",
-                parse_location(location, &state.symbol_tables)?.displayer(&state.symbol_tables)
+                state
+                    .symbol_tables
+                    .parse_location(location)?
+                    .displayer(&state.symbol_tables)
             )))
         },
     )
@@ -524,15 +479,12 @@ fn main() -> Result<(), ReplError> {
             .help("Set a breakpoint or list current breakpoints"),
         |args, state| {
             if let Some(location) = args.get_one::<String>("location") {
-                let location = parse_location(location, &state.symbol_tables)?;
+                let location = state.symbol_tables.parse_location(location)?;
                 if args.get_flag("delete") {
                     let deleted = match location {
-                        Location::Symbol((table, symbol)) => state
-                            .symbol_tables
-                            .get_mut(&table)
-                            .unwrap()
-                            .breakpoints
-                            .shift_remove(&symbol),
+                        Location::Symbol((table, symbol)) => {
+                            state.symbol_tables.delete_breakpoint(&table, &symbol)?
+                        }
                         Location::Address(address) => {
                             state.address_breakpoints.shift_remove(&address)
                         }
@@ -544,31 +496,22 @@ fn main() -> Result<(), ReplError> {
                     }
                 } else {
                     match location {
-                        Location::Symbol((table, symbol)) => state
-                            .symbol_tables
-                            .get_mut(&table)
-                            .unwrap()
-                            .breakpoints
-                            .insert(symbol),
-                        Location::Address(address) => state.address_breakpoints.insert(address),
+                        Location::Symbol((table, symbol)) => {
+                            state.symbol_tables.set_breakpoint(&table, symbol)?;
+                        }
+                        Location::Address(address) => {
+                            state.address_breakpoints.insert(address);
+                        }
                     };
                     Ok(None)
                 }
             } else {
                 let mut out = String::new();
-                for (table_name, table) in &state.symbol_tables {
-                    if !table.breakpoints.is_empty() {
-                        out += &format!(
-                            "{table_name} ({}): \n",
-                            if table.active { "active" } else { "inactive" }
-                        );
-                        for breakpoint in &table.breakpoints {
-                            out += breakpoint;
-                            out += "\n";
-                        }
-                    }
-                }
+                out += &format!("{}", state.symbol_tables.breakpoint_displayer());
                 if !state.address_breakpoints.is_empty() {
+                    if !out.is_empty() {
+                        out += "\n";
+                    }
                     out += "Address breakpoints:\n";
                     for breakpoint in &state.address_breakpoints {
                         out += &format!("{}\n", breakpoint);
@@ -596,31 +539,12 @@ fn main() -> Result<(), ReplError> {
     .run()
 }
 
-fn read_symbol_table(path: &str) -> Result<HashMap<String, Symbol>, Error> {
-    let file = elf::File::open_stream(&mut File::open(path)?).map_err(<Box<dyn StdError>>::from)?;
-    let (symtab, symstrtab) = file
-        .symbol_table()
-        .map_err(<Box<dyn StdError>>::from)?
-        .ok_or(Error::Misc("No symbol table in file"))?;
-    Ok(symtab
-        .iter()
-        .skip(1)
-        .filter(|sym| sym.st_symtype().0 != STT_FILE && sym.st_symtype().0 != STT_SECTION)
-        .map(|sym| {
-            (
-                symstrtab.get(sym.st_name as usize).unwrap().to_string(),
-                Symbol::from(sym),
-            )
-        })
-        .collect::<HashMap<_, _>>())
-}
-
 fn disas_fmt(
     cpu: &mut M68K,
     addr: u32,
     symbol_tables: &SymbolTables,
 ) -> (String, Result<u32, DisassemblyError<BusError>>) {
-    let addr_fmt = if let Some((table, symbol, offset)) = address_to_symbol(addr, symbol_tables) {
+    let addr_fmt = if let Some((table, symbol, offset)) = symbol_tables.address_to_symbol(addr) {
         format!("{}:{} + {} (0x{:x})", table, symbol, offset, addr)
     } else {
         format!("0x{:x}", addr)
@@ -630,63 +554,3 @@ fn disas_fmt(
         Err(e) => (format!("{}: {}\n", addr_fmt, e), Err(e)),
     }
 }
-
-fn parse_location_address(location: &str, symbol_tables: &SymbolTables) -> Result<u32, Error> {
-    parse_location(location, symbol_tables).map(|l| l.addr(symbol_tables))
-}
-
-fn parse_location(location: &str, symbol_tables: &SymbolTables) -> Result<Location, Error> {
-    parse::<u32>(location).map(Location::Address).or_else(|_| {
-        let (mut table_name, symbol_name) = location.split_once(':').unwrap_or(("", location));
-        if table_name.is_empty() {
-            table_name = symbol_tables
-                .iter()
-                .find(|(_, table)| table.symbols.contains_key(symbol_name))
-                .ok_or(Error::InvalidSymbolName)?
-                .0;
-        } else if !symbol_tables
-            .get(table_name)
-            .ok_or(Error::InvalidSymbolTable)?
-            .symbols
-            .contains_key(symbol_name)
-        {
-            return Err(Error::InvalidSymbolName);
-        }
-        Ok(Location::Symbol((
-            table_name.to_string(),
-            symbol_name.to_string(),
-        )))
-    })
-}
-
-fn breakpoint_set_at(
-    addr: u32,
-    symbol_tables: &SymbolTables,
-    address_breakpoints: &IndexSet<u32>,
-) -> bool {
-    address_breakpoints.contains(&addr)
-        || symbol_tables.values().any(|table| {
-            table.active
-                && table
-                    .breakpoints
-                    .iter()
-                    .map(|sym| &table.symbols[sym])
-                    .any(|sym| sym.value() == addr)
-        })
-}
-
-fn address_to_symbol(addr: u32, symbol_tables: &SymbolTables) -> Option<(&String, &String, u32)> {
-    symbol_tables.iter().find_map(|(table_name, table)| {
-        if !table.active {
-            None
-        } else {
-            table
-                .symbols
-                .iter()
-                .filter(|(_, sym)| sym.value() <= addr)
-                .map(|(sym_name, sym)| (sym_name, addr - sym.value()))
-                .min_by_key(|(_, offset)| *offset)
-                .map(|(sym_name, offset)| (table_name, sym_name, offset))
-        }
-    })
-}

src/symbol_table.rs

@@ -1,6 +1,9 @@
-use crate::symbol::Symbol;
+use crate::{error::Error, symbol::Symbol};
+use elf::gabi::{STT_FILE, STT_SECTION};
 use indexmap::IndexSet;
 use std::collections::HashMap;
+use std::error::Error as StdError;
+use std::fs::File;
 
 #[derive(Debug)]
 pub struct SymbolTable {
@@ -18,6 +21,27 @@ impl SymbolTable {
         }
     }
 
+    pub fn read_from_file(path: &str) -> Result<Self, Error> {
+        let file =
+            elf::File::open_stream(&mut File::open(path)?).map_err(<Box<dyn StdError>>::from)?;
+        let (symtab, symstrtab) = file
+            .symbol_table()
+            .map_err(<Box<dyn StdError>>::from)?
+            .ok_or(Error::Misc("No symbol table in file"))?;
+        let symbols = symtab
+            .iter()
+            .skip(1)
+            .filter(|sym| sym.st_symtype().0 != STT_FILE && sym.st_symtype().0 != STT_SECTION)
+            .map(|sym| {
+                (
+                    symstrtab.get(sym.st_name as usize).unwrap().to_string(),
+                    Symbol::from(sym),
+                )
+            })
+            .collect::<HashMap<_, _>>();
+        Ok(Self::new(symbols))
+    }
+
     pub fn update_symbols(&mut self, symbols: HashMap<String, Symbol>) {
         self.breakpoints = self
             .breakpoints
@@ -27,4 +51,18 @@ impl SymbolTable {
             .collect::<IndexSet<_>>();
         self.symbols = symbols;
     }
+
+    pub fn breakpoint_set_at(&self, addr: u32) -> bool {
+        self.breakpoints
+            .iter()
+            .any(|sym| self.symbols[sym].value() == addr)
+    }
+
+    pub fn address_to_symbol(&self, addr: u32) -> Option<(&String, u32)> {
+        self.symbols
+            .iter()
+            .filter(|(_, sym)| sym.value() <= addr)
+            .map(|(sym_name, sym)| (sym_name, addr - sym.value()))
+            .min_by_key(|(_, offset)| *offset)
+    }
 }

src/symbol_tables.rs

@@ -0,0 +1,179 @@
+use std::{fmt::Display, path::Path};
+
+use crate::{error::Error, location::Location, symbol::Symbol, symbol_table::SymbolTable};
+use indexmap::IndexMap;
+use parse_int::parse;
+
+pub struct SymbolDisplayer<'a>(&'a SymbolTables);
+
+impl Display for SymbolDisplayer<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let mut first_table = true;
+        for (table_name, table) in self.0.tables.iter() {
+            if !first_table {
+                f.write_str("\n")?;
+            }
+            f.write_fmt(format_args!(
+                "{table_name} ({}):",
+                if table.active { "active" } else { "inactive" }
+            ))?;
+            for (name, symbol) in table.symbols.iter() {
+                f.write_fmt(format_args!("\n{name}: {symbol}"))?;
+            }
+            first_table = false;
+        }
+        Ok(())
+    }
+}
+
+pub struct BreakpointDisplayer<'a>(&'a SymbolTables);
+
+impl Display for BreakpointDisplayer<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let mut first_table = true;
+        for (table_name, table) in self.0.tables.iter() {
+            if !first_table {
+                f.write_str("\n")?;
+            }
+            if table.breakpoints.is_empty() {
+                continue;
+            }
+            f.write_fmt(format_args!(
+                "{table_name} ({}):",
+                if table.active { "active" } else { "inactive" }
+            ))?;
+            for breakpoint in &table.breakpoints {
+                f.write_fmt(format_args!("\n{breakpoint}"))?;
+            }
+            first_table = false;
+        }
+        Ok(())
+    }
+}
+
+pub struct SymbolTables {
+    pub tables: IndexMap<String, SymbolTable>,
+}
+
+impl SymbolTables {
+    pub fn new() -> Self {
+        Self {
+            tables: IndexMap::new(),
+        }
+    }
+    pub fn breakpoint_set_at(&self, addr: u32) -> bool {
+        self.tables
+            .values()
+            .any(|table| table.active && table.breakpoint_set_at(addr))
+    }
+
+    pub fn address_to_symbol(&self, addr: u32) -> Option<(&String, &String, u32)> {
+        self.tables
+            .iter()
+            .filter(|(_, table)| table.active)
+            .find_map(|(table_name, table)| {
+                table
+                    .address_to_symbol(addr)
+                    .map(|(sym_name, offset)| (table_name, sym_name, offset))
+            })
+    }
+
+    pub fn delete(&mut self, table: &str) -> Result<SymbolTable, Error> {
+        self.tables
+            .shift_remove(table)
+            .ok_or(Error::InvalidSymbolTable)
+    }
+
+    pub fn set_active(&mut self, table: &str, active: bool) -> Result<(), Error> {
+        self.tables
+            .get_mut(table)
+            .ok_or(Error::InvalidSymbolTable)?
+            .active = active;
+        Ok(())
+    }
+
+    pub fn load_table(&mut self, path: &str, append: bool) -> Result<(), Error> {
+        let new_table = SymbolTable::read_from_file(path)?;
+        let table_name = Path::new(&path).file_name().unwrap().to_str().unwrap();
+        if let Some(table) = self.tables.get_mut(table_name) {
+            table.update_symbols(new_table.symbols);
+        } else {
+            self.tables.insert(table_name.to_string(), new_table);
+        };
+        if !append {
+            let table = self.tables.remove(table_name).unwrap();
+            self.tables.clear();
+            self.tables.insert(table_name.to_string(), table);
+        };
+        Ok(())
+    }
+
+    pub fn set_breakpoint(&mut self, table: &str, symbol: String) -> Result<(), Error> {
+        self.tables
+            .get_mut(table)
+            .ok_or(Error::InvalidSymbolTable)?
+            .breakpoints
+            .insert(symbol);
+        Ok(())
+    }
+
+    pub fn delete_breakpoint(&mut self, table: &str, symbol: &str) -> Result<bool, Error> {
+        Ok(self
+            .tables
+            .get_mut(table)
+            .ok_or(Error::InvalidSymbolTable)?
+            .breakpoints
+            .shift_remove(symbol))
+    }
+
+    pub fn symbol_displayer(&self) -> SymbolDisplayer<'_> {
+        SymbolDisplayer(self)
+    }
+
+    pub fn breakpoint_displayer(&self) -> BreakpointDisplayer {
+        BreakpointDisplayer(self)
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.tables.is_empty()
+    }
+
+    pub fn get(&self, table: &str, symbol: &str) -> Result<&Symbol, Error> {
+        self.tables
+            .get(table)
+            .ok_or(Error::InvalidSymbolTable)?
+            .symbols
+            .get(symbol)
+            .ok_or(Error::InvalidSymbolName)
+    }
+
+    pub fn parse_location(&self, location: &str) -> Result<Location, Error> {
+        parse::<u32>(location).map(Location::Address).or_else(|_| {
+            let (mut table_name, symbol_name) = location.split_once(':').unwrap_or(("", location));
+            if table_name.is_empty() {
+                table_name = self
+                    .tables
+                    .iter()
+                    .find(|(_, table)| table.symbols.contains_key(symbol_name))
+                    .ok_or(Error::InvalidSymbolName)?
+                    .0;
+            } else if !self
+                .tables
+                .get(table_name)
+                .ok_or(Error::InvalidSymbolTable)?
+                .symbols
+                .contains_key(symbol_name)
+            {
+                return Err(Error::InvalidSymbolName);
+            }
+            Ok(Location::Symbol((
+                table_name.to_string(),
+                symbol_name.to_string(),
+            )))
+        })
+    }
+
+    pub fn parse_location_address(&self, location: &str) -> Result<u32, Error> {
+        self.parse_location(location).map(|l| l.addr(self))
+    }
+}