Implement core::error::Error for all error types #916
Conversation
uefi/src/data_types/strs.rs
Outdated
| f, | ||
| "{}", | ||
| match self { | ||
| Self::InvalidChar(_usize) => "invalid character", |
There was a problem hiding this comment.
For variants like this that contain extra info, I think we should go ahead and include that in the error message: write!(f, "invalid character at index {index}")
CHANGELOG.md
Outdated
| @@ -15,6 +15,7 @@ | |||
| - `FileSystem::copy` is now more efficient for large files. | |||
| - `MpService::startup_all_aps` and `MpService::startup_this_ap` now accept an | |||
| optional `event` parameter to allow non-blocking operation. | |||
| - Added `core::error::Error` implementations to all error types | |||
There was a problem hiding this comment.
Nit: period at end of sentence.
uefi/src/proto/network/pxe.rs
Outdated
| @@ -1278,6 +1287,15 @@ pub struct TftpError { | |||
| pub error_string: [u8; 127], | |||
| } | |||
|
|
|||
| impl Display for TftpError { | |||
| fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { | |||
There was a problem hiding this comment.
tiny nit: can drop the core:: here since fmt is already imported
uefi/src/data_types/strs.rs
Outdated
| "{}", | ||
| match self { | ||
| Self::InvalidChar(_usize) => "invalid character", | ||
| Self::InteriorNul(_usize) => "encountered null character", |
There was a problem hiding this comment.
For the interior null error, maybe something like "interior null character at index {index}", to clarify that it's specifically an interior null that is the problem (a trailing null is expected and required).
|
Thanks for the contribution, this is a good addition! |
cmoylan
force-pushed
the
implement-error-trait
branch
from
a35e64d to
4dc61a4
Compare
Implemented
core::error::Errorandcore::fmt::Displayfor all error types. Addresses #594I'm fairly new to Rust so there might be some coding conventions I'm not familiar with. I tried to match the style of existing
Displayimplementations. Any and all feedback is appreciated.Checklist