From c758b07b68dd1acfff5ee747c81fec8b8e1c3d1a Mon Sep 17 00:00:00 2001 From: Axel Karjalainen Date: Tue, 18 Nov 2025 23:07:58 +0200 Subject: [PATCH] qdl: switch from anyhow to thiserror This makes it possible for library users to match on errors. Errors are also more descriptive now: Error: I/O error while reading Caused by: 0: libusb error 1: Overflow Compared to what it was before: Error: other error For some reasoning, see https://kazlauskas.me/entries/errors --- Cargo.lock | 26 +++++- cli/src/programfile.rs | 19 ++-- cli/src/util.rs | 4 +- qdl/Cargo.toml | 2 +- qdl/src/lib.rs | 196 ++++++++++++++++++++++++++--------------- qdl/src/parsers.rs | 22 +++-- qdl/src/sahara.rs | 128 +++++++++++++++++++-------- qdl/src/serial.rs | 9 +- qdl/src/types.rs | 26 +++--- qdl/src/usb.rs | 84 ++++++++++-------- 10 files changed, 327 insertions(+), 189 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 76e08e5..4dce0b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -201,7 +201,7 @@ dependencies = [ "crc", "nix", "serde", - "thiserror", + "thiserror 1.0.69", ] [[package]] @@ -322,7 +322,6 @@ name = "qdl" version = "0.1.0" dependencies = [ "anstream", - "anyhow", "bincode", "indexmap", "owo-colors", @@ -331,6 +330,7 @@ dependencies = [ "serde", "serde_repr", "serial2", + "thiserror 2.0.17", "xmltree", ] @@ -449,7 +449,16 @@ version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" dependencies = [ - "thiserror-impl", + "thiserror-impl 1.0.69", +] + +[[package]] +name = "thiserror" +version = "2.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f63587ca0f12b72a0600bcba1d40081f830876000bb46dd2337a3051618f4fc8" +dependencies = [ + "thiserror-impl 2.0.17", ] [[package]] @@ -463,6 +472,17 @@ dependencies = [ "syn", ] +[[package]] +name = "thiserror-impl" +version = "2.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ff15c8ecd7de3849db632e14d18d2571fa09dfc5ed93479bc4485c7a517c913" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "unicode-ident" version = "1.0.18" diff --git a/cli/src/programfile.rs b/cli/src/programfile.rs index afdcae9..8c2cfb1 100644 --- a/cli/src/programfile.rs +++ b/cli/src/programfile.rs @@ -34,7 +34,12 @@ fn parse_read_cmd( let start_sector = attrs.get("start_sector").unwrap().parse::().unwrap(); if checksum_only { - return firehose_checksum_storage(channel, num_sectors, phys_part_idx, start_sector); + return Ok(firehose_checksum_storage( + channel, + num_sectors, + phys_part_idx, + start_sector, + )?); } if !attrs.contains_key("filename") { @@ -42,14 +47,14 @@ fn parse_read_cmd( } let mut outfile = fs::File::create(out_dir.join(attrs.get("filename").unwrap()))?; - firehose_read_storage( + Ok(firehose_read_storage( channel, &mut outfile, num_sectors, slot, phys_part_idx, start_sector, - ) + )?) } fn parse_patch_cmd( @@ -77,7 +82,7 @@ fn parse_patch_cmd( let start_sector = attrs.get("start_sector").unwrap(); let val = attrs.get("value").unwrap(); - firehose_patch( + Ok(firehose_patch( channel, byte_off, slot, @@ -85,7 +90,7 @@ fn parse_patch_cmd( size, start_sector, val, - ) + )?) } const BOOTABLE_PART_NAMES: [&str; 3] = ["xbl", "xbl_a", "sbl1"]; @@ -158,7 +163,7 @@ fn parse_program_cmd( sector_size as i64 * file_sector_offset as i64, ))?; - firehose_program_storage( + Ok(firehose_program_storage( channel, &mut buf, label, @@ -166,7 +171,7 @@ fn parse_program_cmd( slot, phys_part_idx, start_sector, - ) + )?) } // TODO: there's some funny optimizations to make here, such as OoO loading files into memory, or doing things while we're waiting on the device to finish diff --git a/cli/src/util.rs b/cli/src/util.rs index ce6b0f6..419fe8c 100644 --- a/cli/src/util.rs +++ b/cli/src/util.rs @@ -101,12 +101,12 @@ pub fn read_storage_logical_partition( .ok_or(Error::from(ErrorKind::NotFound))? .1; - firehose_read_storage( + Ok(firehose_read_storage( channel, out, (part.ending_lba - part.starting_lba + 1) as usize, slot, phys_part_idx, part.starting_lba as u32, - ) + )?) } diff --git a/qdl/Cargo.toml b/qdl/Cargo.toml index ba300e1..2dbbd33 100644 --- a/qdl/Cargo.toml +++ b/qdl/Cargo.toml @@ -19,7 +19,6 @@ path = "src/lib.rs" [dependencies] anstream = "0.6.15" -anyhow = "1.0" bincode = "1.3.3" indexmap = "2.5.0" owo-colors = "4.1.0" @@ -28,6 +27,7 @@ rusb = { version = "0.9.4", optional = true } serde = { version = "1.0.210", features = ["derive"] } serde_repr = "0.1.19" serial2 = { version = "0.2.28", optional = true } +thiserror = "2.0.17" xmltree = { version = "0.11.0", features = ["attribute-order"] } [features] diff --git a/qdl/src/lib.rs b/qdl/src/lib.rs index e368f01..f5680de 100644 --- a/qdl/src/lib.rs +++ b/qdl/src/lib.rs @@ -1,11 +1,11 @@ // SPDX-License-Identifier: BSD-3-Clause // Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. use anstream::println; -use anyhow::Result; use indexmap::IndexMap; use owo_colors::OwoColorize; use parsers::firehose_parser_ack_nak; use serial::setup_serial_device; +use std::borrow::Cow; use std::cmp::min; use std::io::{Read, Write}; use std::str::{self, FromStr}; @@ -17,10 +17,11 @@ use types::QdlChan; use types::QdlReadWrite; use usb::setup_usb_device; -use anyhow::bail; use pbr::{ProgressBar, Units}; use xmltree::{self, Element, XMLNode}; +use crate::usb::UsbSetupError; + pub mod parsers; pub mod sahara; #[cfg(feature = "serial")] @@ -29,25 +30,74 @@ pub mod types; #[cfg(feature = "usb")] pub mod usb; +#[non_exhaustive] +#[derive(Debug, thiserror::Error)] +pub enum SetupError { + #[error("Failed to set up serial device")] + SerialSetup(std::io::Error), + #[error("Serial port unspecified")] + SerialPortUnspecified, + #[error("Failed to set up USB device")] + UsbSetup(#[from] UsbSetupError), +} + pub fn setup_target_device( backend: QdlBackend, serial_no: Option, - port: Option, -) -> Result> { + serial_port: Option, +) -> Result, SetupError> { match backend { - QdlBackend::Serial => match setup_serial_device(port) { - Ok(d) => Ok(Box::new(d)), - Err(e) => Err(e), - }, - QdlBackend::Usb => match setup_usb_device(serial_no) { - Ok(d) => Ok(Box::new(d)), - Err(e) => Err(e), - }, + QdlBackend::Serial => Ok(Box::new( + setup_serial_device(serial_port.ok_or(SetupError::SerialPortUnspecified)?) + .map_err(SetupError::SerialSetup)?, + )), + QdlBackend::Usb => Ok(Box::new(setup_usb_device(serial_no)?)), } } +#[non_exhaustive] +#[derive(Debug, thiserror::Error)] +pub enum FirehoseError { + #[error("Failed to write XML")] + XmlWrite(#[from] xmltree::Error), + #[error("Failed to parse XML")] + XmlParse(#[from] xmltree::ParseError), + #[error("Got a packet without an XML data tag")] + PacketWithoutXmlDataTag, + + #[error("Firehose programmer requested a restart. Run the program again.")] + RequestedRestart, + #[error("Wrote an unexpected number of bytes: {0}")] + WroteUnexpectedAmount(usize), + + #[error("Got malformed data: {0:?}")] + MalformedData(IndexMap), + + #[error("A request was NAKed (rejected)")] + Nak(#[from] NakError), + + #[error("I/O error")] + IO(#[source] std::io::Error), + + #[error("Device requires protocol version >= {}, the library only supports up to v{}", device_min_version.bright_red(), parsers::FH_PROTO_VERSION_SUPPORTED.bright_blue())] + ProtocolVersionIncompatibility { device_min_version: u32 }, +} + +#[non_exhaustive] +#[derive(Debug, thiserror::Error)] +pub enum NakError { + #[error(" request was NAKed (rejected). Try with qdlrs's verbose_firehose on")] + Configure, + #[error(" request was NAKed (rejected). Did you set the sector size correctly?")] + Program, + #[error("<{0}> request was NAKed (rejected)")] + Request(&'static str), + #[error("{0} was NAKed (rejected)")] + Generic(Cow<'static, str>), +} + /// Wrapper for easily creating Firehose-y XML packets -fn firehose_xml_setup(op: &str, kvps: &[(&str, &str)]) -> anyhow::Result> { +fn firehose_xml_setup(op: &str, kvps: &[(&str, &str)]) -> Result, FirehoseError> { let mut xml = Element::new("data"); let mut op_node = Element::new(op); for kvp in kvps.iter() { @@ -70,8 +120,8 @@ fn firehose_xml_setup(op: &str, kvps: &[(&str, &str)]) -> anyhow::Result /// Main Firehose XML reading function pub fn firehose_read( channel: &mut T, - response_parser: fn(&mut T, &IndexMap) -> Result, -) -> Result { + response_parser: fn(&mut T, &IndexMap) -> Result, +) -> Result { let mut got_any_data = false; let mut pending: Vec = Vec::new(); @@ -79,18 +129,15 @@ pub fn firehose_read( // Use BufRead to peek at available data let available = match channel.fill_buf() { Ok(buf) => buf, - Err(e) => match e.kind() { - // In some cases (like with welcome messages), there's no acking - // and a timeout is the "end of data" marker instead.. - std::io::ErrorKind::TimedOut => { - if got_any_data { - return Ok(FirehoseStatus::Ack); - } else { - return Err(e.into()); - } + Err(e) => { + if e.kind() == std::io::ErrorKind::TimedOut && got_any_data { + // In some cases (like with welcome messages), there's no acking + // and a timeout is the "end of data" marker instead.. + return Ok(FirehoseStatus::Ack); + } else { + return Err(FirehoseError::IO(e)); } - _ => return Err(e.into()), - }, + } }; got_any_data = true; @@ -117,13 +164,7 @@ pub fn firehose_read( // Only parse the XML portion let xml_chunk = &pending[..xml_end]; - let xml = match xmltree::Element::parse(xml_chunk) { - Ok(x) => x, - Err(e) => { - // Consume the bad data and continue - bail!("Failed to parse XML: {}", e); - } - }; + let xml = xmltree::Element::parse(xml_chunk)?; // The current message might have started in "pending", so clear it // now. No need to do this if we're bailing above, as it's a local @@ -135,7 +176,7 @@ pub fn firehose_read( if channel.fh_config().verbose_firehose { println!("{:?}", xml); } - bail!("Got a firehose packet without a data tag"); + return Err(FirehoseError::PacketWithoutXmlDataTag); } // The spec expects there's always a single node only @@ -166,11 +207,11 @@ pub fn firehose_read( // TODO: Use std::intrinsics::unlikely after it exits nightly if e.attributes.get("AttemptRetry").is_some() { - return firehose_read::(channel, response_parser); + return firehose_read(channel, response_parser); } else if e.attributes.get("AttemptRestart").is_some() { // TODO: handle this automagically firehose_reset(channel, &FirehoseResetMode::ResetToEdl, 0)?; - bail!("Firehose requested a restart. Run the program again."); + return Err(FirehoseError::RequestedRestart); } // Pass other nodes to specialized parsers @@ -187,7 +228,7 @@ pub fn firehose_read( } /// Send a Firehose packet -pub fn firehose_write(channel: &mut T, buf: &mut [u8]) -> anyhow::Result<()> { +pub fn firehose_write(channel: &mut T, buf: &mut [u8]) -> Result<(), FirehoseError> { let mut b = buf.to_vec(); // XML can't be n * 512 bytes long by fh spec @@ -207,16 +248,16 @@ pub fn firehose_write(channel: &mut T, buf: &mut [u8]) -> anyhow::Re pub fn firehose_write_getack( channel: &mut T, buf: &mut [u8], - couldnt_what: String, -) -> anyhow::Result<()> { + nak_error: NakError, +) -> Result<(), FirehoseError> { firehose_write(channel, buf)?; - match firehose_read::(channel, firehose_parser_ack_nak) { + match firehose_read(channel, firehose_parser_ack_nak) { Ok(FirehoseStatus::Ack) => Ok(()), Ok(FirehoseStatus::Nak) => { // Assume FH will hang after NAK.. firehose_reset(channel, &FirehoseResetMode::ResetToEdl, 0)?; - Err(anyhow::Error::msg(format!("Couldn't {couldnt_what}"))) + Err(nak_error.into()) } Err(e) => Err(e), } @@ -227,7 +268,7 @@ pub fn firehose_benchmark( channel: &mut T, trials: u32, test_write_perf: bool, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let mut xml = firehose_xml_setup( "benchmark", &[ @@ -243,14 +284,14 @@ pub fn firehose_benchmark( ], )?; - firehose_write_getack(channel, &mut xml, "issue a NOP".to_owned()) + firehose_write_getack(channel, &mut xml, NakError::Request("benchmark")) } /// Send a "Hello"-type packet to the Device pub fn firehose_configure( channel: &mut T, skip_storage_init: bool, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let config = channel.fh_config(); // Spec requirement assert!( @@ -290,10 +331,10 @@ pub fn firehose_configure( } /// Do nothing, hopefully succesfully -pub fn firehose_nop(channel: &mut T) -> anyhow::Result<()> { +pub fn firehose_nop(channel: &mut T) -> Result<(), FirehoseError> { let mut xml = firehose_xml_setup("nop", &[("value", "ping")])?; - firehose_write_getack(channel, &mut xml, "issue a NOP".to_owned()) + firehose_write_getack(channel, &mut xml, NakError::Request("nop")) } /// Get information about the physical partition of a storage medium (e.g. LUN) @@ -301,7 +342,7 @@ pub fn firehose_nop(channel: &mut T) -> anyhow::Result<()> { pub fn firehose_get_storage_info( channel: &mut T, phys_part_idx: u8, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let mut xml = firehose_xml_setup( "getstorageinfo", &[("physical_partition_number", &phys_part_idx.to_string())], @@ -309,7 +350,7 @@ pub fn firehose_get_storage_info( firehose_write(channel, &mut xml)?; - firehose_read::(channel, firehose_parser_ack_nak).and(Ok(())) + firehose_read(channel, firehose_parser_ack_nak).and(Ok(())) } /// Alter Device (TODO: or Host) storage @@ -321,7 +362,7 @@ pub fn firehose_patch( size: u64, start_sector: &str, val: &str, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let mut xml: Vec = firehose_xml_setup( "patch", &[ @@ -339,7 +380,7 @@ pub fn firehose_patch( ], )?; - firehose_write_getack(channel, &mut xml, "patch".to_string()) + firehose_write_getack(channel, &mut xml, NakError::Request("patch")) } /// Peek at memory @@ -348,7 +389,7 @@ pub fn firehose_peek( channel: &mut T, addr: u64, byte_count: u64, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { if channel.fh_config().skip_firehose_log { println!( "{}", @@ -365,7 +406,11 @@ pub fn firehose_peek( ], )?; - firehose_write_getack(channel, &mut xml, format!("peek @ {addr:#x}")) + firehose_write_getack( + channel, + &mut xml, + NakError::Generic(format!(" @ {addr:#x} request").into()), + ) } /// Poke at memory @@ -377,7 +422,7 @@ pub fn firehose_poke( // TODO: byte count is 1..=8 byte_count: u8, val: u64, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let mut xml: Vec = firehose_xml_setup( "poke", &[ @@ -387,7 +432,11 @@ pub fn firehose_poke( ], )?; - firehose_write_getack(channel, &mut xml, format!("peek @ {addr:#x}")) + firehose_write_getack( + channel, + &mut xml, + NakError::Generic(format!(" @ {addr:#x} request").into()), + ) } /// Write to Device storage @@ -399,7 +448,7 @@ pub fn firehose_program_storage( slot: u8, phys_part_idx: u8, start_sector: &str, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let mut sectors_left = num_sectors; let mut xml = firehose_xml_setup( "program", @@ -421,8 +470,8 @@ pub fn firehose_program_storage( firehose_write(channel, &mut xml)?; - if firehose_read::(channel, firehose_parser_ack_nak)? != FirehoseStatus::Ack { - bail!(" was NAKed. Did you set sector-size correctly?"); + if firehose_read(channel, firehose_parser_ack_nak)? != FirehoseStatus::Ack { + return Err(NakError::Program.into()); } let mut pb = ProgressBar::new((sectors_left * channel.fh_config().storage_sector_size) as u64); @@ -446,7 +495,7 @@ pub fn firehose_program_storage( let n = channel.write(&buf).expect("Error sending data"); if n != chunk_size_sectors * channel.fh_config().storage_sector_size { - bail!("Wrote an unexpected number of bytes ({})", n); + return Err(FirehoseError::WroteUnexpectedAmount(n)); } sectors_left -= chunk_size_sectors; @@ -458,8 +507,8 @@ pub fn firehose_program_storage( let _ = channel.write(&[]).expect("Error sending ZLP"); } - if firehose_read::(channel, firehose_parser_ack_nak)? != FirehoseStatus::Ack { - bail!("Failed to complete 'write' op"); + if firehose_read(channel, firehose_parser_ack_nak)? != FirehoseStatus::Ack { + return Err(NakError::Generic("A write operation".into()).into()); } Ok(()) @@ -471,7 +520,7 @@ pub fn firehose_checksum_storage( num_sectors: usize, phys_part_idx: u8, start_sector: u32, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let mut xml = firehose_xml_setup( "getsha256digest", &[ @@ -488,8 +537,8 @@ pub fn firehose_checksum_storage( firehose_write(channel, &mut xml)?; // TODO: figure out some sane way to figure out the timeout - if firehose_read::(channel, firehose_parser_ack_nak)? != FirehoseStatus::Ack { - bail!("Checksum request was NAKed"); + if firehose_read(channel, firehose_parser_ack_nak)? != FirehoseStatus::Ack { + return Err(NakError::Request("getsha256digest").into()); } Ok(()) @@ -503,7 +552,7 @@ pub fn firehose_read_storage( slot: u8, phys_part_idx: u8, start_sector: u32, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let mut bytes_left = num_sectors * channel.fh_config().storage_sector_size; let mut xml = firehose_xml_setup( "read", @@ -521,7 +570,7 @@ pub fn firehose_read_storage( firehose_write(channel, &mut xml)?; if firehose_read(channel, firehose_parser_ack_nak)? != FirehoseStatus::Ack { - bail!("Read request was NAKed"); + return Err(NakError::Request("read").into()); } let mut pb = ProgressBar::new(bytes_left as u64); @@ -541,7 +590,7 @@ pub fn firehose_read_storage( } last_read_was_zero_len = false; - let _ = out.write(&buf[..n])?; + let _ = out.write(&buf[..n]).map_err(FirehoseError::IO)?; bytes_left -= n; pb.add(n as u64); @@ -549,11 +598,11 @@ pub fn firehose_read_storage( if !last_read_was_zero_len && channel.fh_config().backend == QdlBackend::Usb { // Issue a dummy read to drain the queue - let _ = channel.read(&mut [])?; + let _ = channel.read(&mut []).map_err(FirehoseError::IO)?; } if firehose_read(channel, firehose_parser_ack_nak)? != FirehoseStatus::Ack { - bail!("Failed to complete 'read' op"); + return Err(NakError::Generic("A read operation".into()).into()); } Ok(()) @@ -564,7 +613,7 @@ pub fn firehose_reset( channel: &mut T, mode: &FirehoseResetMode, delay_in_sec: u32, -) -> anyhow::Result<()> { +) -> Result<(), FirehoseError> { let mut xml = firehose_xml_setup( "power", &[ @@ -580,11 +629,14 @@ pub fn firehose_reset( ], )?; - firehose_write_getack(channel, &mut xml, "reset the Device".to_owned()) + firehose_write_getack(channel, &mut xml, NakError::Request("power")) } /// Mark a physical storage partition as bootable -pub fn firehose_set_bootable(channel: &mut T, drive_idx: u8) -> anyhow::Result<()> { +pub fn firehose_set_bootable( + channel: &mut T, + drive_idx: u8, +) -> Result<(), FirehoseError> { let mut xml = firehose_xml_setup( "setbootablestoragedrive", &[("value", &drive_idx.to_string())], @@ -593,7 +645,7 @@ pub fn firehose_set_bootable(channel: &mut T, drive_idx: u8) -> anyh firehose_write_getack( channel, &mut xml, - format!("set partition {drive_idx} as bootable"), + NakError::Generic(format!("An operation to set partition {drive_idx} as bootable").into()), ) } diff --git a/qdl/src/parsers.rs b/qdl/src/parsers.rs index 997eb95..72fb33e 100644 --- a/qdl/src/parsers.rs +++ b/qdl/src/parsers.rs @@ -3,15 +3,15 @@ use indexmap::IndexMap; -use anyhow::bail; use owo_colors::OwoColorize; use crate::{ - FirehoseResetMode, FirehoseStatus, QdlChan, firehose_configure, firehose_read, firehose_reset, + FirehoseError, FirehoseResetMode, FirehoseStatus, NakError, QdlChan, firehose_configure, + firehose_read, firehose_reset, }; /// The highest protocol version currently supported by the library -const FH_PROTO_VERSION_SUPPORTED: u32 = 1; +pub(crate) const FH_PROTO_VERSION_SUPPORTED: u32 = 1; // Parsers are kept separate for more flexibility (e.g. log replay analysis) @@ -19,12 +19,12 @@ const FH_PROTO_VERSION_SUPPORTED: u32 = 1; pub fn firehose_parser_ack_nak( _: &mut T, attrs: &IndexMap, -) -> Result { +) -> Result { let val = attrs.get("value").to_owned(); match &val.unwrap()[..] { "ACK" => Ok(FirehoseStatus::Ack), "NAK" => Ok(FirehoseStatus::Nak), - _ => bail!("Got malformed data: {:?}", attrs), + _ => Err(FirehoseError::MalformedData(attrs.clone())), } } @@ -32,7 +32,7 @@ pub fn firehose_parser_ack_nak( pub fn firehose_parser_configure_response( channel: &mut T, attrs: &IndexMap, -) -> Result { +) -> Result { if let Ok(status) = firehose_parser_ack_nak(channel, attrs) { // The device can't handle that big of a buffer and it auto-reconfigures to the max it can if status == FirehoseStatus::Nak { @@ -40,7 +40,7 @@ pub fn firehose_parser_configure_response( channel.mut_fh_config().send_buffer_size = val.parse::().unwrap(); } else { firehose_reset(channel, &FirehoseResetMode::ResetToEdl, 0)?; - bail!("firehose failed, try again with --verbose-firehose") + return Err(FirehoseError::Nak(NakError::Configure)); } } } @@ -62,11 +62,9 @@ pub fn firehose_parser_configure_response( println!("Found protocol version {}", version.bright_blue()); if min_version_supported < FH_PROTO_VERSION_SUPPORTED { - bail!( - "Device requires protocol version >= {}, the library only supports up to v{}", - min_version_supported.bright_red(), - FH_PROTO_VERSION_SUPPORTED.bright_blue() - ); + return Err(FirehoseError::ProtocolVersionIncompatibility { + device_min_version: min_version_supported, + }); } // TODO: MaxPayloadSizeFromTargetInBytes seems useless when xfers are abstracted through libusb diff --git a/qdl/src/sahara.rs b/qdl/src/sahara.rs index a09a0f6..1bc2d15 100644 --- a/qdl/src/sahara.rs +++ b/qdl/src/sahara.rs @@ -9,16 +9,60 @@ use std::{ fs::File, io::{Read, Write}, mem::{self, size_of_val}, + str::Utf8Error, + string::FromUtf8Error, }; -use anyhow::{Result, anyhow, bail}; - use bincode::serialize; use serde::{self, Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; use crate::types::{QdlBackend, QdlChan}; +#[non_exhaustive] +#[derive(Debug, thiserror::Error)] +pub enum SaharaError { + #[error("Failed to write XML")] + XmlWrite(#[from] xmltree::Error), + #[error("Failed to parse XML")] + XmlParse(#[from] xmltree::ParseError), + #[error("Got a packet without an XML data tag")] + PacketWithoutXmlDataTag, + + #[error("Firehose programmer requested a restart. Run the program again.")] + RequestedRestart, + #[error("Wrote an unexpected number of bytes: {0}")] + WroteUnexpectedAmount(usize), + + #[error("Missing Sahara command in sahara_run")] + MissingSaharaCommand, + + #[error("Attempted OOB read {pos} > {bound}")] + AttemptedOOBRead { pos: usize, bound: usize }, + + #[error("I/O error while reading")] + ReadIO(#[source] std::io::Error), + #[error("I/O error while writing")] + WriteIO(#[source] std::io::Error), + #[error("File I/O error while dumping regions to local disk")] + DumpRegionsFileIO(#[source] std::io::Error), + + #[error("Failed to decode UTF-8")] + Utf8(#[from] Utf8Error), + + #[error("Bincode error")] + Bincode(#[from] bincode::Error), + + #[error("Malformed packet, too short: {buf:?}")] + PacketTooShort { buf: Vec }, +} + +impl From for SaharaError { + fn from(value: FromUtf8Error) -> Self { + Self::Utf8(value.utf8_error()) + } +} + const SAHARA_STATUS_SUCCESS: u32 = 0; #[derive(Copy, Clone, Debug, PartialEq, Deserialize_repr, Serialize_repr)] @@ -223,20 +267,23 @@ pub fn sahara_send_img_to_device( image_idx: u64, image_offset: u64, image_len: u64, -) -> Result { +) -> Result { let image = if img_arr.len() == 1 { 0 } else { image_idx }; let buf = &mut img_arr[image as usize]; - if (image_offset + image_len) as usize > buf.len() { - bail!( - "Attempted OOB read {} > {}", - image_offset + image_len, - buf.len() - ); + + let start = image_offset as usize; + let end = (image_offset + image_len) as usize; + + if start > buf.len() { + return Err(SaharaError::AttemptedOOBRead { + pos: start, + bound: buf.len(), + }); } channel - .write(&buf[image_offset as usize..(image_offset + image_len) as usize]) - .map_err(|e| e.into()) + .write(&buf[start..end]) + .map_err(SaharaError::WriteIO) } fn sahara_send_generic( @@ -244,7 +291,7 @@ fn sahara_send_generic( cmd: SaharaCmd, body: SaharaPacketBody, body_len: usize, -) -> Result { +) -> Result { let pkt = SaharaPacket { cmd, len: (size_of_val(&cmd) + size_of::() + body_len) as u32, @@ -253,11 +300,14 @@ fn sahara_send_generic( channel .write(&serialize(&pkt).expect("Error serializing packet")) - .map_err(|e| e.into()) + .map_err(SaharaError::WriteIO) } const SAHARA_VERSION: u32 = 2; -pub fn sahara_send_hello_rsp(channel: &mut T, mode: SaharaMode) -> Result { +pub fn sahara_send_hello_rsp( + channel: &mut T, + mode: SaharaMode, +) -> Result { let data = HelloResp { ver: SAHARA_VERSION, compatible: 1, @@ -279,7 +329,7 @@ pub fn sahara_send_hello_rsp(channel: &mut T, mode: SaharaMode) ) } -pub fn sahara_send_done(channel: &mut T) -> Result { +pub fn sahara_send_done(channel: &mut T) -> Result { let data = DoneReq {}; sahara_send_generic( @@ -293,7 +343,7 @@ pub fn sahara_send_done(channel: &mut T) -> Result { pub fn sahara_send_cmd_exec( channel: &mut T, command: SaharaCmdModeCmd, -) -> Result { +) -> Result { sahara_send_generic( channel, SaharaCmd::SaharaExecute, @@ -305,7 +355,7 @@ pub fn sahara_send_cmd_exec( pub fn sahara_send_cmd_data( channel: &mut T, command: SaharaCmdModeCmd, -) -> Result { +) -> Result { sahara_send_generic( channel, SaharaCmd::SaharaExecuteData, @@ -314,7 +364,7 @@ pub fn sahara_send_cmd_data( ) } -pub fn sahara_reset(channel: &mut T) -> Result { +pub fn sahara_reset(channel: &mut T) -> Result { let data = ResetReq {}; sahara_send_generic( @@ -328,7 +378,7 @@ pub fn sahara_reset(channel: &mut T) -> Result( channel: &mut T, mode: SaharaMode, -) -> Result { +) -> Result { let data = SwitchMode { mode }; sahara_send_generic( @@ -344,7 +394,7 @@ pub fn sahara_get_ramdump_tbl( addr: u64, len: u64, verbose: bool, -) -> Result, anyhow::Error> { +) -> Result, SaharaError> { let data = ReadMem64Req { addr, len }; sahara_send_generic( @@ -359,7 +409,7 @@ pub fn sahara_get_ramdump_tbl( let mut tbl = Vec::::with_capacity(num_chunks); let mut buf = vec![0u8; len as usize]; - channel.read_exact(&mut buf)?; + channel.read_exact(&mut buf).map_err(SaharaError::ReadIO)?; if verbose { println!("Available images:"); @@ -388,7 +438,7 @@ fn sahara_dump_region( channel: &mut T, entry: RamdumpTable64, output: &mut impl Write, -) -> Result<()> { +) -> Result<(), SaharaError> { let mut pb = ProgressBar::new(entry.len); pb.show_time_left = true; pb.message(&format!( @@ -412,9 +462,9 @@ fn sahara_dump_region( SaharaPacketBody::ReadMem64Req(data), size_of_val(&data), )?; - channel.flush()?; + channel.flush().map_err(SaharaError::WriteIO)?; - bytes_read += channel.read(&mut buf)?; + bytes_read += channel.read(&mut buf).map_err(SaharaError::ReadIO)?; // Issue a dummy read to consume the ZLP if channel.fh_config().backend == QdlBackend::Usb && buf.len().is_multiple_of(512) { @@ -422,7 +472,7 @@ fn sahara_dump_region( } pb.set(bytes_read as u64); - let _ = output.write(&buf)?; + let _ = output.write(&buf).map_err(SaharaError::WriteIO)?; } Ok(()) @@ -432,14 +482,14 @@ pub fn sahara_dump_regions( channel: &mut T, dump_tbl: Vec, regions_to_dump: Vec, -) -> Result<()> { +) -> Result<(), SaharaError> { // Make all of them lowercase for better UX let regions_to_dump = regions_to_dump .iter() .map(|rname| rname.to_ascii_lowercase()) .collect::>(); - std::fs::create_dir_all("ramdump/")?; + std::fs::create_dir_all("ramdump/").map_err(SaharaError::DumpRegionsFileIO)?; let filtered_list: Vec = match regions_to_dump.len() { // Dump everything with save_pref == true if no argument was provided 0 => dump_tbl @@ -469,7 +519,8 @@ pub fn sahara_dump_regions( .to_str()? .to_owned(); - let mut f = File::create(std::path::Path::new(&format!("ramdump/{fname}")))?; + let mut f = File::create(std::path::Path::new(&format!("ramdump/{fname}"))) + .map_err(SaharaError::DumpRegionsFileIO)?; sahara_dump_region(channel, entry, &mut f)?; } @@ -483,11 +534,11 @@ pub fn sahara_run( images: &mut [Vec], filenames: Vec, verbose: bool, -) -> Result> { +) -> Result, SaharaError> { let mut buf = vec![0; 4096]; loop { - let bytes_read = channel.read(&mut buf[..])?; + let bytes_read = channel.read(&mut buf[..]).map_err(SaharaError::ReadIO)?; let pkt = sahara_parse_packet(&buf[..bytes_read], verbose)?; let pktsize = size_of_val(&pkt.cmd) + size_of_val(&pkt.len); @@ -543,9 +594,9 @@ pub fn sahara_run( SaharaCmd::SaharaCommandReady => { assert_eq!(pkt.len as usize, pktsize); match sahara_command { - Some(cmd) => sahara_send_cmd_exec(channel, cmd), - None => bail!("Missing sahara command"), - }?; + Some(cmd) => sahara_send_cmd_exec(channel, cmd)?, + None => return Err(SaharaError::MissingSaharaCommand), + }; } SaharaCmd::SaharaExecuteResp => { if let SaharaPacketBody::ExecResp(resp) = pkt.body { @@ -554,7 +605,7 @@ pub fn sahara_run( // Indicate we're ready to receive the requested amount of data sahara_send_cmd_data(channel, resp.command)?; - let resp_len = channel.read(&mut resp_buf)?; + let resp_len = channel.read(&mut resp_buf).map_err(SaharaError::ReadIO)?; assert_eq!(resp_len, resp.len as usize); // Got everything we want, exit command mode @@ -595,16 +646,15 @@ pub fn sahara_run( } } -fn sahara_parse_packet(buf: &[u8], verbose: bool) -> Result { +fn sahara_parse_packet(buf: &[u8], verbose: bool) -> Result { let (cmd, rest) = buf .split_first_chunk::<4>() - .ok_or_else(|| anyhow!("Malformed packet, too short: {buf:?}"))?; + .ok_or_else(|| SaharaError::PacketTooShort { buf: buf.clone() })?; let (len, args) = rest .split_first_chunk::<4>() - .ok_or_else(|| anyhow!("Malformed packet, too short: {buf:?}"))?; + .ok_or_else(|| SaharaError::PacketTooShort { buf: buf.clone() })?; - let cmd = bincode::deserialize::(cmd) - .unwrap_or_else(|_| panic!("Got unknown command {}", u32::from_le_bytes(*cmd))); + let cmd = bincode::deserialize::(cmd)?; let ret = SaharaPacket { cmd, diff --git a/qdl/src/serial.rs b/qdl/src/serial.rs index abdcc84..0e9e559 100644 --- a/qdl/src/serial.rs +++ b/qdl/src/serial.rs @@ -1,6 +1,5 @@ // SPDX-License-Identifier: BSD-3-Clause // Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. -use anyhow::{Result, bail}; use serial2::{self, SerialPort}; use std::io::{BufRead, Read, Write}; @@ -62,12 +61,8 @@ impl BufRead for QdlSerialConfig { impl QdlReadWrite for QdlSerialConfig {} -pub fn setup_serial_device(dev_path: Option) -> Result { - if dev_path.is_none() { - bail!("Serial port path unspecified"); - } - - let serport = SerialPort::open(dev_path.unwrap(), |mut settings: serial2::Settings| { +pub fn setup_serial_device(dev_path: String) -> Result { + let serport = SerialPort::open(dev_path, |mut settings: serial2::Settings| { settings.set_raw(); settings.set_baud_rate(115200)?; Ok(settings) diff --git a/qdl/src/types.rs b/qdl/src/types.rs index c63de98..c594ea1 100644 --- a/qdl/src/types.rs +++ b/qdl/src/types.rs @@ -2,15 +2,18 @@ // Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. use std::{ fmt::Display, - io::{BufRead, ErrorKind, Read, Write}, + io::{BufRead, Read, Write}, str::FromStr, }; -use anyhow::{Error, bail}; use owo_colors::OwoColorize; use crate::firehose_reset; +#[derive(Debug, thiserror::Error)] +#[error("Unknown variant: {0}")] +pub struct UnknownStringVariant(&'static str); + /// Common respones indicating success/failure respectively #[derive(Copy, Clone, Debug, PartialEq)] #[repr(u32)] @@ -26,22 +29,23 @@ pub enum QdlBackend { } impl FromStr for QdlBackend { - type Err = anyhow::Error; + type Err = UnknownStringVariant; fn from_str(s: &str) -> Result { match s { "serial" => Ok(QdlBackend::Serial), "usb" => Ok(QdlBackend::Usb), - _ => bail!("Unknown backend"), + _ => Err(UnknownStringVariant("Firehose backend")), } } } impl Default for QdlBackend { fn default() -> Self { - match cfg!(target_os = "windows") { - true => QdlBackend::Serial, - false => QdlBackend::Usb, + if cfg!(target_os = "windows") { + QdlBackend::Serial + } else { + QdlBackend::Usb } } } @@ -179,7 +183,7 @@ pub enum FirehoseStorageType { } impl FromStr for FirehoseStorageType { - type Err = Error; + type Err = UnknownStringVariant; fn from_str(input: &str) -> Result { match input { @@ -188,7 +192,7 @@ impl FromStr for FirehoseStorageType { "nand" => Ok(FirehoseStorageType::Nand), "nvme" => Ok(FirehoseStorageType::Nvme), "spinor" => Ok(FirehoseStorageType::Spinor), - _ => bail!("Unknown storage type"), + _ => Err(UnknownStringVariant("Firehose storage type")), } } } @@ -213,14 +217,14 @@ pub enum FirehoseResetMode { } impl FromStr for FirehoseResetMode { - type Err = anyhow::Error; + type Err = UnknownStringVariant; fn from_str(s: &str) -> Result { match s { "edl" => Ok(FirehoseResetMode::ResetToEdl), "system" => Ok(FirehoseResetMode::Reset), "off" => Ok(FirehoseResetMode::Off), - _ => Err(std::io::Error::from(ErrorKind::InvalidInput).into()), + _ => Err(UnknownStringVariant("Firehose reset mode")), } } } diff --git a/qdl/src/usb.rs b/qdl/src/usb.rs index 485ddc0..d282a84 100644 --- a/qdl/src/usb.rs +++ b/qdl/src/usb.rs @@ -1,9 +1,8 @@ // SPDX-License-Identifier: BSD-3-Clause // Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. -use anyhow::{Context, Result, bail}; use rusb::{self, Device, DeviceHandle, GlobalContext}; use std::{ - io::{BufRead, Error, ErrorKind, Read, Write}, + io::{BufRead, Read, Write}, time::Duration, }; @@ -81,31 +80,46 @@ const INTF_DESC_PROTO_CODES: [u8; 3] = [0x10, 0x11, 0xFF]; fn find_usb_handle_by_sn( devices: &mut dyn Iterator>, serial_no: String, -) -> Result> { - let mut dev_handle: Option> = None; - +) -> Result, UsbSetupError> { for d in devices { - let dh = d.open()?; + let dh = d.open().map_err(UsbSetupError::Open)?; - let prod_str = dh.read_product_string_ascii(&d.device_descriptor().unwrap())?; + let prod_str = dh + .read_product_string_ascii(&d.device_descriptor().unwrap()) + .map_err(UsbSetupError::MiscRusb)?; let sn = &prod_str[prod_str.find("_SN:").unwrap() + "_SN:".len()..]; if sn.eq_ignore_ascii_case(&serial_no) { - dev_handle = Some(dh); - break; + return Ok(dh); } } - match dev_handle { - Some(h) => Ok(h), - None => bail!( - "Found no devices in EDL mode with serial number {}", - serial_no - ), - } + Err(UsbSetupError::NoDevicesSerial { serial_no }) +} + +#[non_exhaustive] +#[derive(Debug, thiserror::Error)] +pub enum UsbSetupError { + #[error("libusb error")] + MiscRusb(#[source] rusb::Error), + + #[error("Failed to find devices in EDL mode with serial number {serial_no}")] + NoDevicesSerial { serial_no: String }, + #[error("Failed to find devices in EDL mode")] + NoDevices, + + #[error("Missing {0}")] + Missing(&'static str), + + #[error("Failed to enumerate devices")] + Enumeration(#[source] rusb::Error), + #[error("Failed to open a device")] + Open(#[source] rusb::Error), + #[error("Failed to claim interface {0}")] + ClaimInterface(u8, #[source] rusb::Error), } -pub fn setup_usb_device(serial_no: Option) -> Result { - let rusb_devices = rusb::devices()?; +pub fn setup_usb_device(serial_no: Option) -> Result { + let rusb_devices = rusb::devices().map_err(UsbSetupError::Enumeration)?; let mut devices = rusb_devices .iter() .filter(|d: &rusb::Device| { @@ -114,17 +128,18 @@ pub fn setup_usb_device(serial_no: Option) -> Result { }); let dev_handle = match serial_no { - Some(s) => find_usb_handle_by_sn(&mut devices, s), + Some(s) => find_usb_handle_by_sn(&mut devices, s)?, None => { - let Some(d) = devices.next() else { - bail!("Found no devices in EDL mode") - }; - d.open().map_err(|e| rusb_err_xlate(e).into()) + let d = devices.next().ok_or(UsbSetupError::NoDevices)?; + d.open().map_err(UsbSetupError::Open)? } - }?; + }; // TODO: is there always precisely one interface like this? - let cfg_desc = dev_handle.device().active_config_descriptor()?; + let cfg_desc = dev_handle + .device() + .active_config_descriptor() + .map_err(UsbSetupError::MiscRusb)?; let intf_desc = cfg_desc .interfaces() .next() @@ -136,28 +151,28 @@ pub fn setup_usb_device(serial_no: Option) -> Result { && INTF_DESC_PROTO_CODES.contains(&d.protocol_code()) && d.num_endpoints() >= 2 }) - .ok_or::(Error::from(ErrorKind::NotFound).into())?; + .ok_or(UsbSetupError::Missing("matching interface"))?; let in_ep = intf_desc .endpoint_descriptors() .find(|e| { e.direction() == rusb::Direction::In && e.transfer_type() == rusb::TransferType::Bulk }) - .unwrap() + .ok_or(UsbSetupError::Missing("USB endpoint"))? .address(); let out_ep = intf_desc .endpoint_descriptors() .find(|e| { e.direction() == rusb::Direction::Out && e.transfer_type() == rusb::TransferType::Bulk }) - .unwrap() + .ok_or(UsbSetupError::Missing("USB endpoint"))? .address(); // Make sure we can actually poke at the device dev_handle.set_auto_detach_kernel_driver(true).ok(); dev_handle .claim_interface(intf_desc.interface_number()) - .with_context(|| format!("Couldn't claim interface{}", intf_desc.interface_number()))?; + .map_err(|err| UsbSetupError::ClaimInterface(intf_desc.interface_number(), err))?; Ok(QdlUsbConfig { dev_handle, in_ep, @@ -168,12 +183,11 @@ pub fn setup_usb_device(serial_no: Option) -> Result { }) } +#[derive(Debug, thiserror::Error)] +#[error("libusb error")] +struct RusbErrorWrap(#[source] rusb::Error); + // TODO: fix this upstream? pub fn rusb_err_xlate(e: rusb::Error) -> std::io::Error { - std::io::Error::from(match e { - rusb::Error::Timeout => std::io::ErrorKind::TimedOut, - rusb::Error::Access => std::io::ErrorKind::PermissionDenied, - rusb::Error::NoDevice => std::io::ErrorKind::NotConnected, - _ => std::io::ErrorKind::Other, - }) + std::io::Error::other(RusbErrorWrap(e)) }