-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add error handling in AML bytecode creation code #4870
Conversation
Right now, we include a few panics and assertions in the code that generates AML bytecode for our ACPI tables. Change the AML methods to explicitly return a Result type and substitute these panics and assertions with error types. Also, change some type methods that now need to return a Result as well. Signed-off-by: Babis Chalios <[email protected]>
We were calling stuff "Cachable" instead of "Cacheable". Signed-off-by: Babis Chalios <[email protected]>
acpi_tables::aml::AddressSpace<T> type is describing an address space with a minimum address `min: T` and a maximum address `max: T`. Currently, we do not perform any checks on these values when creating the AddressSpace objects. This means that the starting address `min` can be bigger than the last address `max`, which can cause underflows when calculating the length of this address space, i.e. `max - min + 1`. Add a check in the constructor methods of AddressSpace objects and return an error when `min > max`. Signed-off-by: Babis Chalios <[email protected]>
4b28f91
to
cda1442
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4870 +/- ##
==========================================
- Coverage 84.08% 84.05% -0.03%
==========================================
Files 251 251
Lines 28060 28096 +36
==========================================
+ Hits 23594 23616 +22
- Misses 4466 4480 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I am not sure errors are better in this case. If we fail in acpi tables creation there is no way to recover. Having code to panic or having it to propagate all the way to Update: |
Changes
Change the
acpi_tables::aml::Aml
trait methods to returnResult
types and adapt all the relevant implementations. Also, add a check in the methods that createacpi_tables::aml::AddressSpace<T>
types, to check that the address ranges are reasonable, i.e. first address is not bigger than the last address of the address space.Reason
The first change allows us to substitute a handful of
panic!
s andassert!
s with proper error handling. The second change avoids a potential underflow when we calculate the length of the address space.License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md
.PR Checklist
PR.
CHANGELOG.md
.TODO
s link to an issue.contribution quality standards.
rust-vmm
.