Refactoring: remove the FirmwareImage trait and eliminate the elf module (#802)

* Use `ElfFile` instead of `dyn FirmwareImage` where able

* Convert the `image_format` module to a directory

* Eliminate the `elf` module, move its contents to `image_format`

* Eliminate the `FirmwareImage` trait

* Update `CHANGELOG.md`

Author: jessebraham

CHANGELOG.md

@@ -38,8 +38,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Removed
 
 - Removed the `libudev` feature (#742)
-- The `FirmwareImage` trait no longer includes the `segments_with_load_addresses` function (#796)
 - Removed the `flasher::parse_partition_table` function (#798)
+- The `FirmwareImage` trait has been removed (#802)
+- The `elf` module has been removed, and its contents moved to the `image_format` module (#802)
 
 ## [3.3.0] - 2025-01-13
 

espflash/src/cli/mod.rs

@@ -607,9 +607,9 @@ pub fn save_elf_as_image(
     if merge {
         // To get a chip revision, the connection is needed
         // For simplicity, the revision None is used
-        let image =
-            chip.into_target()
-                .get_flash_image(&elf, flash_data.clone(), None, xtal_freq)?;
+        let image = chip
+            .into_target()
+            .get_flash_image(elf, flash_data.clone(), None, xtal_freq)?;
 
         display_image_size(image.app_size(), image.part_size());
 
@@ -643,7 +643,7 @@ pub fn save_elf_as_image(
     } else {
         let image = chip
             .into_target()
-            .get_flash_image(&elf, flash_data, None, xtal_freq)?;
+            .get_flash_image(elf, flash_data, None, xtal_freq)?;
 
         display_image_size(image.app_size(), image.part_size());
 

espflash/src/flasher/mod.rs

@@ -31,14 +31,14 @@ use crate::{
         Connection,
         Port,
     },
-    elf::{FirmwareImage, Segment},
     error::{ConnectionError, ResultExt},
     flasher::stubs::{
         FlashStub,
         CHIP_DETECT_MAGIC_REG_ADDR,
         DEFAULT_TIMEOUT,
         EXPECTED_STUB_HANDSHAKE,
     },
+    image_format::{ram_segments, rom_segments, Segment},
 };
 use crate::{
     error::{ElfError, Error},
@@ -1012,19 +1012,19 @@ impl Flasher {
         mut progress: Option<&mut dyn ProgressCallbacks>,
     ) -> Result<(), Error> {
         let elf = ElfFile::new(elf_data).map_err(ElfError::from)?;
-        if elf.rom_segments(self.chip).next().is_some() {
+        if rom_segments(self.chip, &elf).next().is_some() {
             return Err(Error::ElfNotRamLoadable);
         }
 
         let mut target = self.chip.ram_target(
-            Some(elf.entry()),
+            Some(elf.header.pt2.entry_point() as u32),
             self.chip
                 .into_target()
                 .max_ram_block_size(&mut self.connection)?,
         );
         target.begin(&mut self.connection).flashing()?;
 
-        for segment in elf.ram_segments(self.chip) {
+        for segment in ram_segments(self.chip, &elf) {
             target
                 .write_segment(&mut self.connection, segment, &mut progress)
                 .flashing()?;
@@ -1057,7 +1057,7 @@ impl Flasher {
         let image =
             self.chip
                 .into_target()
-                .get_flash_image(&elf, flash_data, chip_revision, xtal_freq)?;
+                .get_flash_image(elf, flash_data, chip_revision, xtal_freq)?;
 
         // When the `cli` feature is enabled, display the image size information.
         #[cfg(feature = "cli")]

espflash/src/image_format.rs renamed to espflash/src/image_format/esp_idf.rs

@@ -6,9 +6,10 @@ use bytemuck::{bytes_of, from_bytes, Pod, Zeroable};
 use esp_idf_part::{AppType, DataType, Partition, PartitionTable, SubType, Type};
 use log::debug;
 use sha2::{Digest, Sha256};
+use xmas_elf::ElfFile;
 
+use super::{ram_segments, rom_segments, Segment};
 use crate::{
-    elf::{FirmwareImage, Segment},
     flasher::{FlashData, FlashFrequency, FlashMode, FlashSize},
     targets::{Chip, Esp32Params},
     Error,
@@ -119,7 +120,7 @@ pub struct IdfBootloaderFormat<'a> {
 
 impl<'a> IdfBootloaderFormat<'a> {
     pub fn new(
-        image: &'a dyn FirmwareImage<'a>,
+        elf: ElfFile<'a>,
         chip: Chip,
         flash_data: FlashData,
         params: Esp32Params,
@@ -190,17 +191,16 @@ impl<'a> IdfBootloaderFormat<'a> {
         // write the header of the app
         // use the same settings as the bootloader
         // just update the entry point
-        header.entry = image.entry();
-
+        header.entry = elf.header.pt2.entry_point() as u32;
         header.wp_pin = WP_PIN_DISABLED;
         header.chip_id = params.chip_id;
         header.min_chip_rev_full = flash_data.min_chip_rev;
         header.append_digest = 1;
 
         let mut data = bytes_of(&header).to_vec();
 
-        let flash_segments: Vec<_> = merge_adjacent_segments(image.rom_segments(chip).collect());
-        let mut ram_segments: Vec<_> = merge_adjacent_segments(image.ram_segments(chip).collect());
+        let flash_segments: Vec<_> = merge_adjacent_segments(rom_segments(chip, &elf).collect());
+        let mut ram_segments: Vec<_> = merge_adjacent_segments(ram_segments(chip, &elf).collect());
 
         let mut checksum = ESP_CHECKSUM_MAGIC;
         let mut segment_count = 0;

espflash/src/elf.rs renamed to espflash/src/image_format/mod.rs

@@ -1,4 +1,4 @@
-//! ELF (Executable and Linkable Format) file operations
+//! Binary application image formats
 
 use std::{
     borrow::Cow,
@@ -13,58 +13,10 @@ use xmas_elf::{
     ElfFile,
 };
 
+pub use self::esp_idf::IdfBootloaderFormat;
 use crate::targets::Chip;
 
-/// Operations for working with firmware images
-pub trait FirmwareImage<'a> {
-    /// Firmware image entry point
-    fn entry(&self) -> u32;
-
-    /// Firmware image segments
-    fn segments(&'a self) -> Box<dyn Iterator<Item = Segment<'a>> + 'a>;
-
-    /// Firmware image ROM segments
-    fn rom_segments(&'a self, chip: Chip) -> Box<dyn Iterator<Item = Segment<'a>> + 'a> {
-        Box::new(
-            self.segments()
-                .filter(move |segment| chip.into_target().addr_is_flash(segment.addr)),
-        )
-    }
-
-    /// Firmware image RAM segments
-    fn ram_segments(&'a self, chip: Chip) -> Box<dyn Iterator<Item = Segment<'a>> + 'a> {
-        Box::new(
-            self.segments()
-                .filter(move |segment| !chip.into_target().addr_is_flash(segment.addr)),
-        )
-    }
-}
-
-impl<'a> FirmwareImage<'a> for ElfFile<'a> {
-    fn entry(&self) -> u32 {
-        self.header.pt2.entry_point() as u32
-    }
-
-    fn segments(&'a self) -> Box<dyn Iterator<Item = Segment<'a>> + 'a> {
-        Box::new(
-            self.section_iter()
-                .filter(|header| {
-                    header.size() > 0
-                        && header.get_type() == Ok(ShType::ProgBits)
-                        && header.offset() > 0
-                        && header.address() > 0
-                })
-                .flat_map(move |header| {
-                    let addr = header.address() as u32;
-                    let data = match header.get_data(self) {
-                        Ok(SectionData::Undefined(data)) => data,
-                        _ => return None,
-                    };
-                    Some(Segment::new(addr, data))
-                }),
-        )
-    }
-}
+mod esp_idf;
 
 /// A segment of code from the source ELF
 #[derive(Default, Clone, Eq)]
@@ -192,3 +144,38 @@ impl Ord for Segment<'_> {
         self.addr.cmp(&other.addr)
     }
 }
+
+/// Returns an iterator over all RAM segments for a given chip and ELF file.
+pub(crate) fn ram_segments<'a>(
+    chip: Chip,
+    elf: &'a ElfFile<'a>,
+) -> Box<dyn Iterator<Item = Segment<'a>> + 'a> {
+    Box::new(segments(elf).filter(move |segment| !chip.into_target().addr_is_flash(segment.addr)))
+}
+
+/// Returns an iterator over all ROM segments for a given chip and ELF file.
+pub(crate) fn rom_segments<'a>(
+    chip: Chip,
+    elf: &'a ElfFile<'a>,
+) -> Box<dyn Iterator<Item = Segment<'a>> + 'a> {
+    Box::new(segments(elf).filter(move |segment| chip.into_target().addr_is_flash(segment.addr)))
+}
+
+fn segments<'a>(elf: &'a ElfFile<'a>) -> Box<dyn Iterator<Item = Segment<'a>> + 'a> {
+    Box::new(
+        elf.section_iter()
+            .filter(|header| {
+                header.size() > 0
+                    && header.get_type() == Ok(ShType::ProgBits)
+                    && header.offset() > 0
+                    && header.address() > 0
+            })
+            .flat_map(move |header| {
+                let addr = header.address() as u32;
+                match header.get_data(elf) {
+                    Ok(SectionData::Undefined(data)) => Some(Segment::new(addr, data)),
+                    _ => None,
+                }
+            }),
+    )
+}

espflash/src/lib.rs

@@ -29,10 +29,7 @@
 #![cfg_attr(docsrs, feature(doc_cfg))]
 #![deny(missing_debug_implementations, rust_2018_idioms)]
 
-pub use self::{
-    elf::{FirmwareImage, Segment},
-    error::Error,
-};
+pub use self::{error::Error, image_format::Segment};
 
 #[cfg(feature = "serialport")]
 #[cfg_attr(docsrs, doc(cfg(feature = "serialport")))]
@@ -41,7 +38,6 @@ pub mod flasher;
 pub mod image_format;
 pub mod targets;
 
-mod elf;
 mod error;
 
 // Command-line interface

espflash/src/targets/esp32.rs

@@ -1,9 +1,10 @@
 use std::ops::Range;
 
+use xmas_elf::ElfFile;
+
 #[cfg(feature = "serialport")]
 use crate::{connection::Connection, targets::bytes_to_mac_addr};
 use crate::{
-    elf::FirmwareImage,
     flasher::{FlashData, FlashFrequency},
     image_format::IdfBootloaderFormat,
     targets::{Chip, Esp32Params, ReadEFuse, SpiRegisters, Target, XtalFrequency},
@@ -154,7 +155,7 @@ impl Target for Esp32 {
 
     fn get_flash_image<'a>(
         &self,
-        image: &'a dyn FirmwareImage<'a>,
+        elf: ElfFile<'a>,
         flash_data: FlashData,
         _chip_revision: Option<(u32, u32)>,
         xtal_freq: XtalFrequency,
@@ -183,7 +184,7 @@ impl Target for Esp32 {
             booloader,
         );
 
-        IdfBootloaderFormat::new(image, Chip::Esp32, flash_data, params)
+        IdfBootloaderFormat::new(elf, Chip::Esp32, flash_data, params)
     }
 
     #[cfg(feature = "serialport")]

espflash/src/targets/esp32c2.rs

@@ -1,11 +1,11 @@
 use std::{collections::HashMap, ops::Range};
 
 use log::debug;
+use xmas_elf::ElfFile;
 
 #[cfg(feature = "serialport")]
 use crate::{connection::Connection, targets::bytes_to_mac_addr};
 use crate::{
-    elf::FirmwareImage,
     flasher::{FlashData, FlashFrequency},
     image_format::IdfBootloaderFormat,
     targets::{Chip, Esp32Params, ReadEFuse, SpiRegisters, Target, XtalFrequency},
@@ -90,7 +90,7 @@ impl Target for Esp32c2 {
 
     fn get_flash_image<'a>(
         &self,
-        image: &'a dyn FirmwareImage<'a>,
+        elf: ElfFile<'a>,
         flash_data: FlashData,
         _chip_revision: Option<(u32, u32)>,
         xtal_freq: XtalFrequency,
@@ -121,7 +121,7 @@ impl Target for Esp32c2 {
             booloader,
         );
 
-        IdfBootloaderFormat::new(image, Chip::Esp32c2, flash_data, params)
+        IdfBootloaderFormat::new(elf, Chip::Esp32c2, flash_data, params)
     }
 
     #[cfg(feature = "serialport")]

espflash/src/targets/esp32c3.rs

@@ -1,9 +1,10 @@
 use std::ops::Range;
 
+use xmas_elf::ElfFile;
+
 #[cfg(feature = "serialport")]
 use crate::connection::Connection;
 use crate::{
-    elf::FirmwareImage,
     flasher::{FlashData, FlashFrequency},
     image_format::IdfBootloaderFormat,
     targets::{Chip, Esp32Params, ReadEFuse, SpiRegisters, Target, XtalFrequency},
@@ -78,7 +79,7 @@ impl Target for Esp32c3 {
 
     fn get_flash_image<'a>(
         &self,
-        image: &'a dyn FirmwareImage<'a>,
+        elf: ElfFile<'a>,
         flash_data: FlashData,
         _chip_revision: Option<(u32, u32)>,
         xtal_freq: XtalFrequency,
@@ -90,7 +91,7 @@ impl Target for Esp32c3 {
             });
         }
 
-        IdfBootloaderFormat::new(image, Chip::Esp32c3, flash_data, PARAMS)
+        IdfBootloaderFormat::new(elf, Chip::Esp32c3, flash_data, PARAMS)
     }
 
     fn spi_registers(&self) -> SpiRegisters {

espflash/src/targets/esp32c6.rs

@@ -1,9 +1,10 @@
 use std::ops::Range;
 
+use xmas_elf::ElfFile;
+
 #[cfg(feature = "serialport")]
 use crate::connection::Connection;
 use crate::{
-    elf::FirmwareImage,
     flasher::{FlashData, FlashFrequency},
     image_format::IdfBootloaderFormat,
     targets::{Chip, Esp32Params, ReadEFuse, SpiRegisters, Target, XtalFrequency},
@@ -73,7 +74,7 @@ impl Target for Esp32c6 {
 
     fn get_flash_image<'a>(
         &self,
-        image: &'a dyn FirmwareImage<'a>,
+        elf: ElfFile<'a>,
         flash_data: FlashData,
         _chip_revision: Option<(u32, u32)>,
         xtal_freq: XtalFrequency,
@@ -85,7 +86,7 @@ impl Target for Esp32c6 {
             });
         }
 
-        IdfBootloaderFormat::new(image, Chip::Esp32c6, flash_data, PARAMS)
+        IdfBootloaderFormat::new(elf, Chip::Esp32c6, flash_data, PARAMS)
     }
 
     fn spi_registers(&self) -> SpiRegisters {