Centralize SmbiosError to EFI Status conversion using From impl#1384
Conversation
✅ QEMU Validation PassedAll QEMU validation jobs completed successfully.
Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/22957134501 Boot Time to EFI Shell
This comment was automatically generated by the Patina QEMU PR Validation Post workflow. |
github-actions
bot
added
impact:non-functional
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
kat-perez
force-pushed
the
centralize-smbios-error-conversion
branch
from
c481b9a to
d5f0d25
Compare
Use the existing From<SmbiosError> for EfiError impl chain instead of manual match arms in protocol.rs FFI functions. Map RecordTooSmall and StringPoolTooSmall to BufferTooSmall for accurate error semantics. Closes OpenDevicePartnership#1163
kat-perez
force-pushed
the
centralize-smbios-error-conversion
branch
from
d5f0d25 to
fa79bfb
Compare
There was a problem hiding this comment.
I like this.
You could make this even more concise if you are interested, however. Right now you are doing a SmbiosError->EfiError->efi::Status conversion, however you could move your patina::error::EfiError::from(e).into() logic into a impl From<SmbiosError> for efi::Status so that these different match statements are just Err(e) => e.into().
|
Whoops auto merge merged this. I guess I should have left a blocking comment to be manually resolved lmao. |
|
@Javagedes oops yeah, I can put up another PR with your suggestion |
## Description Adds `impl From<SmbiosError> for efi::Status` to streamline error conversion in FFI protocol functions, simplifying `add_ext`, `update_string_ext`, and `remove_ext` from `patina::error::EfiError::from(e).into()` to `e.into()`. Implements feedback from @Javagedes on #1384 (which auto-merged before this could be addressed). - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested `cargo make all` passes — all tests, clippy, fmt, deny, and doc checks clean. ## Integration Instructions N/A
3 participants
Description
Centralize the
SmbiosErrortoefi::Statusconversion in protocol.rs by using the existingFrom<SmbiosError> for EfiErrorimpl and theEfiError→efi::Statusconversion chain, instead of manual match arms in each FFI function (add_ext,update_string_ext,remove_ext).Additionally,
RecordTooSmallandStringPoolTooSmallare remapped fromEfiError::InvalidParametertoEfiError::BufferTooSmallfor more accurate error semantics.Closes #1163
How This Was Tested
Ran
cargo test -p patina_smbios— all 134 tests pass, including updated assertions for the newBufferTooSmallmappings intest_smbios_error_to_efi_error_conversion.Integration Instructions
N/A