Study alternate designs for warnings

[HadrienG2]

As discussed in #51 , I think EFI warnings should be exposed as part of regular results, rather than as errors. We should, however, make them must_use to hint the user into checking them.

Another option would be to replace core::Result with a more elaborate enum that uses ternary logic:

enum EfiResult<T> {
    Ok(T),
    Warn(T, Status),
    Err(Status)
}

But we would then need to duplicate a small amount of Result code and API and to think about how this will integrate with the Try (?) operator. I think a promising avenue for the latter would be to print a warning to the logs as part of into_result.

In any case, a problem to be resolved is that EFI purposely does not define which operations may throw a warning and which may not. So any solution must be generalizable to the whole EFI API surface with minimal boilerplate.

[HadrienG2]

Started playing around with this a bit. The EfiResult ternary logic seems to model the problem most accurately, but it has several big things going against it:

• It is highly confusing for someone who is used to core::Result
• It is not compatible with core::Result, which periodically comes to bite as we directly use it in places
• It is not compatible with standard Ok() and Err() notation, one must use EfiResult::Ok() etc
• One must essentially reinvent the whole Result implementation (map_err, ok_or, and friends)

For this reason, I'm now experimenting with an alternate design which is less elegant, but much easier to integrate in the existing Rust error handling ecosystem:

// All names are open to bikeshedding
enum Completion<T> {
    Success(T),
    Warning(T, Status),
}

type Result<T> = core::Result<Completion<T>, Status>;

With some careful From and Into impls, it looks like this could be made ergonomic enough... but I need to experiment some more to be sure.

[GabrielMajeri]

@HadrienG2 Interesting. First, would it be a better idea to have a separate Success T and Warning T? As in, does a function returning a warning also return a T, or occasionally a different type?

How is this going to fit in with the existing Try trait implementation we're using for Status?
In other words, how do we plan to use the existing raw UEFI functions, returning Status, to ergonomically create these Results?

[HadrienG2]

As far as I know, the semantics of warnings in UEFI are that the operation mostly completed as intended (and therefore mostly produced the expected result), but some unforeseen event happened along the way. Existing standard warnings describe...

• An unknown character not being rendered correctly in text output
• Some scheduled operations (writes, deletions...) not occuring at the time where a file is closed
• Output data being truncated to the size of a user-provider buffer
• Outdated data being returned from a cache instead of the up-to-date version
• A system reset being necessary for some operation to be fully processed

From this perspective, it makes sense that the result type be homogeneous. The warning does not fundamentally change the result, it only notifies that some non-fatal failure occurred along the way.

I'm currently trying to port the uefi-rs codebase to this design to see how well it works in practice. With added support to Status's Try implementation and into_with() method, together with some support methods to Completion (e.g. map()), it actually works surprisingly well. In particular, being able to get errors out of the way with Try and keep the Completion around to accumulate further warnings is convenient in methods that do multiple UEFI operations in a row like the high-level protocol ops.

There are two limitations that emerge though, but they are not specific to this design:

• Some interfaces provide no way to report warnings, in which case they can only be logged.
• Some methods call into UEFI multiple times, and therefore may in principle produce multiple warnings. Since the proposed design can only store one warning (which I think cannot be addressed cleanly without allocation), if multiple warnings occur, some of them can only be logged and only the latest warning will be emitted.

Once I'm reasonably satisfied with the port, I will submit a PR, it will be easier to discuss around that 😄

[HadrienG2]

On a side note, this Completion<T> design is morally equivalent to a (T, Option<Status>), which could be expressed more memory-efficiently as a (T, Status) where Status::SUCCESS replaces None. The only reason why I'm using an enum here is to clarify semantics, especially in pattern-matching usage.