Skip to content

Commit 745bef6

Browse files
JurajSadeljessebraham
JurajSadel
and
jessebraham
authored
Avoid leaking inner error types vol.2 (#905)
* Avoid leaking inner error types vol.2 * remove redundant impl * Update espflash/src/error.rs Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com> --------- Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com>
1 parent 188cd4e commit 745bef6

File tree

3 files changed

+61
-40
lines changed

3 files changed

+61
-40
lines changed

‎espflash/src/connection/mod.rs

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ impl Connection {
104104
}
105105
}
106106

107-
Err(Error::Connection(ConnectionError::ConnectionFailed))
107+
Err(Error::Connection(Box::new(
108+
ConnectionError::ConnectionFailed,
109+
)))
108110
}
109111

110112
/// Try to connect to a device.
@@ -130,10 +132,10 @@ impl Connection {
130132
let read_bytes = self.serial.read(&mut buff)? as u32;
131133

132134
if read_bytes != available_bytes {
133-
return Err(Error::Connection(ConnectionError::ReadMismatch(
135+
return Err(Error::Connection(Box::new(ConnectionError::ReadMismatch(
134136
available_bytes,
135137
read_bytes,
136-
)));
138+
))));
137139
}
138140
read_bytes
139141
} else {
@@ -172,15 +174,17 @@ impl Connection {
172174

173175
if boot_log_detected {
174176
if download_mode {
175-
return Err(Error::Connection(ConnectionError::NoSyncReply));
177+
return Err(Error::Connection(Box::new(ConnectionError::NoSyncReply)));
176178
} else {
177-
return Err(Error::Connection(ConnectionError::WrongBootMode(
179+
return Err(Error::Connection(Box::new(ConnectionError::WrongBootMode(
178180
boot_mode.to_string(),
179-
)));
181+
))));
180182
}
181183
}
182184

183-
Err(Error::Connection(ConnectionError::ConnectionFailed))
185+
Err(Error::Connection(Box::new(
186+
ConnectionError::ConnectionFailed,
187+
)))
184188
}
185189

186190
/// Try to sync with the device for a given timeout.
@@ -196,17 +200,17 @@ impl Connection {
196200
Some(response) if response.return_op == CommandType::Sync as u8 => {
197201
if response.status == 1 {
198202
connection.flush().ok();
199-
return Err(Error::RomError(RomError::new(
203+
return Err(Error::RomError(Box::new(RomError::new(
200204
CommandType::Sync,
201205
RomErrorKind::from(response.error),
202-
)));
206+
))));
203207
}
204208
}
205209
_ => {
206-
return Err(Error::RomError(RomError::new(
210+
return Err(Error::RomError(Box::new(RomError::new(
207211
CommandType::Sync,
208212
RomErrorKind::InvalidMessage,
209-
)));
213+
))));
210214
}
211215
}
212216
}
@@ -465,10 +469,10 @@ impl Connection {
465469
Some(response) if response.return_op == ty as u8 => {
466470
return if response.status != 0 {
467471
let _error = self.flush();
468-
Err(Error::RomError(RomError::new(
472+
Err(Error::RomError(Box::new(RomError::new(
469473
command.command_type(),
470474
RomErrorKind::from(response.error),
471-
)))
475+
))))
472476
} else {
473477
// Check if the response is a Vector and strip header (first 8 bytes)
474478
// https://github.com/espressif/esptool/blob/749d1ad/esptool/loader.py#L481
@@ -486,7 +490,9 @@ impl Connection {
486490
_ => continue,
487491
}
488492
}
489-
Err(Error::Connection(ConnectionError::ConnectionFailed))
493+
Err(Error::Connection(Box::new(
494+
ConnectionError::ConnectionFailed,
495+
)))
490496
}
491497

492498
/// Read a register command with a timeout.

‎espflash/src/error.rs

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,10 @@ pub enum Error {
187187
ParseChipRevError { chip_rev: String },
188188

189189
#[error("Error while connecting to device")]
190-
#[diagnostic(transparent)]
191-
Connection(#[source] ConnectionError),
190+
Connection(CoreError),
192191

193192
#[error("Communication error while flashing device")]
194-
#[diagnostic(transparent)]
195-
Flashing(#[source] ConnectionError),
193+
Flashing(CoreError),
196194

197195
#[error("Supplied ELF image is not valid")]
198196
#[diagnostic(
@@ -203,20 +201,17 @@ pub enum Error {
203201

204202
#[error("Supplied ELF image contains an invalid application descriptor")]
205203
#[diagnostic(code(espflash::invalid_app_descriptor))]
206-
InvalidAppDescriptor(#[from] AppDescriptorError),
204+
InvalidAppDescriptor(CoreError),
207205

208206
#[error("The bootloader returned an error")]
209207
#[cfg(feature = "serialport")]
210-
#[diagnostic(transparent)]
211-
RomError(#[from] RomError),
208+
RomError(CoreError),
212209

213210
#[error("The selected partition does not exist in the partition table")]
214-
#[diagnostic(transparent)]
215-
MissingPartition(#[from] MissingPartition),
211+
MissingPartition(CoreError),
216212

217213
#[error("The partition table is missing or invalid")]
218-
#[diagnostic(transparent)]
219-
MissingPartitionTable(#[from] MissingPartitionTable),
214+
MissingPartitionTable(CoreError),
220215

221216
#[cfg(feature = "cli")]
222217
#[error(transparent)]
@@ -271,6 +266,14 @@ pub enum Error {
271266
AppDescriptorNotPresent(String),
272267
}
273268

269+
#[cfg(feature = "serialport")]
270+
impl From<SlipError> for Error {
271+
fn from(err: SlipError) -> Self {
272+
let conn_err: ConnectionError = err.into(); // uses first impl
273+
Self::Connection(Box::new(conn_err))
274+
}
275+
}
276+
274277
#[cfg(feature = "serialport")]
275278
impl From<io::Error> for Error {
276279
fn from(err: io::Error) -> Self {
@@ -286,13 +289,6 @@ impl From<serialport::Error> for Error {
286289
}
287290
}
288291

289-
#[cfg(feature = "serialport")]
290-
impl From<SlipError> for Error {
291-
fn from(err: SlipError) -> Self {
292-
Self::Connection(err.into())
293-
}
294-
}
295-
296292
impl From<TryFromSliceError> for Error {
297293
fn from(err: TryFromSliceError) -> Self {
298294
Self::TryFromSlice(Box::new(err))
@@ -305,6 +301,12 @@ impl From<object::Error> for Error {
305301
}
306302
}
307303

304+
impl From<AppDescriptorError> for Error {
305+
fn from(err: AppDescriptorError) -> Self {
306+
Self::InvalidAppDescriptor(Box::new(err))
307+
}
308+
}
309+
308310
#[cfg(feature = "cli")]
309311
impl From<dialoguer::Error> for Error {
310312
fn from(err: dialoguer::Error) -> Self {
@@ -331,6 +333,7 @@ pub enum AppDescriptorError {
331333
/// Connection-related errors.
332334
#[derive(Debug, Diagnostic, Error)]
333335
#[non_exhaustive]
336+
#[cfg(feature = "serialport")]
334337
pub enum ConnectionError {
335338
#[error("Failed to connect to the device")]
336339
#[diagnostic(
@@ -604,13 +607,21 @@ impl<T> ResultExt for Result<T, Error> {
604607
}
605608

606609
fn for_command(self, command: CommandType) -> Self {
607-
match self {
608-
Err(Error::Connection(ConnectionError::Timeout(_))) => {
609-
Err(Error::Connection(ConnectionError::Timeout(command.into())))
610-
}
611-
Err(Error::Flashing(ConnectionError::Timeout(_))) => {
612-
Err(Error::Flashing(ConnectionError::Timeout(command.into())))
610+
fn remap_if_timeout(err: CoreError, command: CommandType) -> CoreError {
611+
match err.downcast::<ConnectionError>() {
612+
Ok(boxed_ce) => match *boxed_ce {
613+
ConnectionError::Timeout(_) => {
614+
Box::new(ConnectionError::Timeout(command.into()))
615+
}
616+
other => Box::new(other),
617+
},
618+
Err(orig) => orig,
613619
}
620+
}
621+
622+
match self {
623+
Err(Error::Connection(err)) => Err(Error::Connection(remap_if_timeout(err, command))),
624+
Err(Error::Flashing(err)) => Err(Error::Flashing(remap_if_timeout(err, command))),
614625
res => res,
615626
}
616627
}

‎espflash/src/flasher/mod.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,7 +787,9 @@ impl Flasher {
787787

788788
match self.connection.read(EXPECTED_STUB_HANDSHAKE.len())? {
789789
Some(resp) if resp == EXPECTED_STUB_HANDSHAKE.as_bytes() => Ok(()),
790-
_ => Err(Error::Connection(ConnectionError::InvalidStubHandshake)),
790+
_ => Err(Error::Connection(Box::new(
791+
ConnectionError::InvalidStubHandshake,
792+
))),
791793
}?;
792794

793795
// Re-detect chip to check stub is up
@@ -953,7 +955,9 @@ impl Flasher {
953955
}
954956
i += 1;
955957
if i > 10 {
956-
return Err(Error::Connection(ConnectionError::Timeout(command.into())));
958+
return Err(Error::Connection(Box::new(ConnectionError::Timeout(
959+
command.into(),
960+
))));
957961
}
958962
}
959963

0 commit comments

Comments
 (0)