Addressed PR feedback

gimli-rs · Dec 11, 2023 · 9950e2c · 9950e2c
1 parent b7f87df
commit 9950e2c
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 71 deletions.
diff --git a/src/read/any.rs b/src/read/any.rs
@@ -14,18 +14,15 @@ use crate::read::pe;
 use crate::read::wasm;
 #[cfg(feature = "xcoff")]
 use crate::read::xcoff;
-use crate::{
-    read::{
-        self, Architecture, BinaryFormat, CodeView, ComdatKind, CompressedData,
-        CompressedFileRange, Error, Export, FileFlags, FileKind, Import, Object, ObjectComdat,
-        ObjectKind, ObjectMap, ObjectSection, ObjectSegment, ObjectSymbol, ObjectSymbolTable,
-        ReadRef, Relocation, Result, SectionFlags, SectionIndex, SectionKind, SegmentFlags,
-        SymbolFlags, SymbolIndex, SymbolKind, SymbolMap, SymbolMapName, SymbolScope, SymbolSection,
-    },
-    SubArchitecture,
+use crate::read::{
+    self, Architecture, BinaryFormat, CodeView, ComdatKind, CompressedData, CompressedFileRange,
+    Error, Export, FileFlags, FileKind, Import, Object, ObjectComdat, ObjectKind, ObjectMap,
+    ObjectSection, ObjectSegment, ObjectSymbol, ObjectSymbolTable, ReadRef, Relocation, Result,
+    SectionFlags, SectionIndex, SectionKind, SegmentFlags, SymbolFlags, SymbolIndex, SymbolKind,
+    SymbolMap, SymbolMapName, SymbolScope, SymbolSection,
 };
 #[allow(unused_imports)]
-use crate::{AddressSize, Endian, Endianness};
+use crate::{AddressSize, Endian, Endianness, SubArchitecture};
 
 /// Evaluate an expression on the contents of a file format enum.
 ///

diff --git a/src/write/coff/object.rs b/src/write/coff/object.rs
@@ -268,20 +268,18 @@ impl<'a> Object<'a> {
 
         // Start writing.
         writer.write_file_header(writer::FileHeader {
-            machine: match self.architecture {
-                Architecture::Arm => coff::IMAGE_FILE_MACHINE_ARMNT,
-                Architecture::Aarch64 => match self.sub_architecture {
-                    None => coff::IMAGE_FILE_MACHINE_ARM64,
-                    Some(SubArchitecture::Arm64EC) => coff::IMAGE_FILE_MACHINE_ARM64EC,
-                    #[allow(unreachable_patterns)]
-                    _ => unimplemented!(),
-                },
-                Architecture::I386 => coff::IMAGE_FILE_MACHINE_I386,
-                Architecture::X86_64 => coff::IMAGE_FILE_MACHINE_AMD64,
+            machine: match (self.architecture, self.sub_architecture) {
+                (Architecture::Arm, None) => coff::IMAGE_FILE_MACHINE_ARMNT,
+                (Architecture::Aarch64, None) => coff::IMAGE_FILE_MACHINE_ARM64,
+                (Architecture::Aarch64, Some(SubArchitecture::Arm64EC)) => {
+                    coff::IMAGE_FILE_MACHINE_ARM64EC
+                }
+                (Architecture::I386, None) => coff::IMAGE_FILE_MACHINE_I386,
+                (Architecture::X86_64, None) => coff::IMAGE_FILE_MACHINE_AMD64,
                 _ => {
                     return Err(Error(format!(
-                        "unimplemented architecture {:?}",
-                        self.architecture
+                        "unimplemented architecture {:?} with sub-architecture {:?}",
+                        self.architecture, self.sub_architecture
                     )));
                 }
             },

diff --git a/src/write/elf/object.rs b/src/write/elf/object.rs
@@ -281,34 +281,34 @@ impl<'a> Object<'a> {
 
         // Start writing.
         let e_type = elf::ET_REL;
-        let e_machine = match self.architecture {
-            Architecture::Aarch64 => elf::EM_AARCH64,
-            Architecture::Aarch64_Ilp32 => elf::EM_AARCH64,
-            Architecture::Arm => elf::EM_ARM,
-            Architecture::Avr => elf::EM_AVR,
-            Architecture::Bpf => elf::EM_BPF,
-            Architecture::Csky => elf::EM_CSKY,
-            Architecture::I386 => elf::EM_386,
-            Architecture::X86_64 => elf::EM_X86_64,
-            Architecture::X86_64_X32 => elf::EM_X86_64,
-            Architecture::Hexagon => elf::EM_HEXAGON,
-            Architecture::LoongArch64 => elf::EM_LOONGARCH,
-            Architecture::Mips => elf::EM_MIPS,
-            Architecture::Mips64 => elf::EM_MIPS,
-            Architecture::Msp430 => elf::EM_MSP430,
-            Architecture::PowerPc => elf::EM_PPC,
-            Architecture::PowerPc64 => elf::EM_PPC64,
-            Architecture::Riscv32 => elf::EM_RISCV,
-            Architecture::Riscv64 => elf::EM_RISCV,
-            Architecture::S390x => elf::EM_S390,
-            Architecture::Sbf => elf::EM_SBF,
-            Architecture::Sharc => elf::EM_SHARC,
-            Architecture::Sparc64 => elf::EM_SPARCV9,
-            Architecture::Xtensa => elf::EM_XTENSA,
+        let e_machine = match (self.architecture, self.sub_architecture) {
+            (Architecture::Aarch64, None) => elf::EM_AARCH64,
+            (Architecture::Aarch64_Ilp32, None) => elf::EM_AARCH64,
+            (Architecture::Arm, None) => elf::EM_ARM,
+            (Architecture::Avr, None) => elf::EM_AVR,
+            (Architecture::Bpf, None) => elf::EM_BPF,
+            (Architecture::Csky, None) => elf::EM_CSKY,
+            (Architecture::I386, None) => elf::EM_386,
+            (Architecture::X86_64, None) => elf::EM_X86_64,
+            (Architecture::X86_64_X32, None) => elf::EM_X86_64,
+            (Architecture::Hexagon, None) => elf::EM_HEXAGON,
+            (Architecture::LoongArch64, None) => elf::EM_LOONGARCH,
+            (Architecture::Mips, None) => elf::EM_MIPS,
+            (Architecture::Mips64, None) => elf::EM_MIPS,
+            (Architecture::Msp430, None) => elf::EM_MSP430,
+            (Architecture::PowerPc, None) => elf::EM_PPC,
+            (Architecture::PowerPc64, None) => elf::EM_PPC64,
+            (Architecture::Riscv32, None) => elf::EM_RISCV,
+            (Architecture::Riscv64, None) => elf::EM_RISCV,
+            (Architecture::S390x, None) => elf::EM_S390,
+            (Architecture::Sbf, None) => elf::EM_SBF,
+            (Architecture::Sharc, None) => elf::EM_SHARC,
+            (Architecture::Sparc64, None) => elf::EM_SPARCV9,
+            (Architecture::Xtensa, None) => elf::EM_XTENSA,
             _ => {
                 return Err(Error(format!(
-                    "unimplemented architecture {:?}",
-                    self.architecture
+                    "unimplemented architecture {:?} with sub-architecture {:?}",
+                    self.architecture, self.sub_architecture
                 )));
             }
         };

diff --git a/src/write/macho.rs b/src/write/macho.rs
@@ -470,20 +470,24 @@ impl<'a> Object<'a> {
             .map_err(|_| Error(String::from("Cannot allocate buffer")))?;
 
         // Write file header.
-        let (cputype, mut cpusubtype) = match self.architecture {
-            Architecture::Arm => (macho::CPU_TYPE_ARM, macho::CPU_SUBTYPE_ARM_ALL),
-            Architecture::Aarch64 => (macho::CPU_TYPE_ARM64, macho::CPU_SUBTYPE_ARM64_ALL),
-            Architecture::Aarch64_Ilp32 => {
+        let (cputype, mut cpusubtype) = match (self.architecture, self.sub_architecture) {
+            (Architecture::Arm, None) => (macho::CPU_TYPE_ARM, macho::CPU_SUBTYPE_ARM_ALL),
+            (Architecture::Aarch64, None) => (macho::CPU_TYPE_ARM64, macho::CPU_SUBTYPE_ARM64_ALL),
+            (Architecture::Aarch64_Ilp32, None) => {
                 (macho::CPU_TYPE_ARM64_32, macho::CPU_SUBTYPE_ARM64_32_V8)
             }
-            Architecture::I386 => (macho::CPU_TYPE_X86, macho::CPU_SUBTYPE_I386_ALL),
-            Architecture::X86_64 => (macho::CPU_TYPE_X86_64, macho::CPU_SUBTYPE_X86_64_ALL),
-            Architecture::PowerPc => (macho::CPU_TYPE_POWERPC, macho::CPU_SUBTYPE_POWERPC_ALL),
-            Architecture::PowerPc64 => (macho::CPU_TYPE_POWERPC64, macho::CPU_SUBTYPE_POWERPC_ALL),
+            (Architecture::I386, None) => (macho::CPU_TYPE_X86, macho::CPU_SUBTYPE_I386_ALL),
+            (Architecture::X86_64, None) => (macho::CPU_TYPE_X86_64, macho::CPU_SUBTYPE_X86_64_ALL),
+            (Architecture::PowerPc, None) => {
+                (macho::CPU_TYPE_POWERPC, macho::CPU_SUBTYPE_POWERPC_ALL)
+            }
+            (Architecture::PowerPc64, None) => {
+                (macho::CPU_TYPE_POWERPC64, macho::CPU_SUBTYPE_POWERPC_ALL)
+            }
             _ => {
                 return Err(Error(format!(
-                    "unimplemented architecture {:?}",
-                    self.architecture
+                    "unimplemented architecture {:?} with sub-architecture {:?}",
+                    self.architecture, self.sub_architecture
                 )));
             }
         };

diff --git a/src/write/mod.rs b/src/write/mod.rs
@@ -1,6 +1,6 @@
 //! Interface for writing object files.
 
-use alloc::borrow::{Cow, ToOwned};
+use alloc::borrow::Cow;
 use alloc::string::String;
 use alloc::vec::Vec;
 use core::{fmt, result, str};
@@ -126,17 +126,8 @@ impl<'a> Object<'a> {
     }
 
     /// Specify the sub-architecture.
-    pub fn set_sub_architecture(
-        &mut self,
-        sub_architecture: Option<SubArchitecture>,
-    ) -> Result<()> {
-        match (self.architecture, sub_architecture) {
-            (Architecture::Aarch64, Some(SubArchitecture::Arm64EC)) | (_, None) => {
-                self.sub_architecture = sub_architecture;
-                Ok(())
-            }
-            _ => Err(Error("Unsupported sub-architecture".to_owned())),
-        }
+    pub fn set_sub_architecture(&mut self, sub_architecture: Option<SubArchitecture>) {
+        self.sub_architecture = sub_architecture;
     }
 
     /// Return the current mangling setting.

diff --git a/tests/round_trip/mod.rs b/tests/round_trip/mod.rs
@@ -29,7 +29,7 @@ fn coff_any() {
     .copied()
     {
         let mut object = write::Object::new(BinaryFormat::Coff, arch, Endianness::Little);
-        object.set_sub_architecture(sub_arch).unwrap();
+        object.set_sub_architecture(sub_arch);
 
         object.add_file_symbol(b"file.c".to_vec());