find_handles(): should return empty vec instead of Status::NotFound

[phip1611]

It is not intuitive to expose that error to users when we already have a vec that can be checked for emptiness.

[nicholasbishop]

I'm a little torn on this; I agree that this would have been a better way to do things from the start, but I worry about the upgrade story. Since the function signatures of find_handles and locate_handles are unchanged there won't be any compilation errors to warn users that something has changed, but code that used to be correct may start behaving incorrectly at runtime. For example:

let handles = boot::find_handles::<Output>()?;
let output_handle = handles[0];

Prior to this change, if no handle is found an error will be propagated to the caller. After the change, the program would instead panic on handles[0].

Of course ideally everyone will carefully read the changelog and examine their code for problems, but in practice I suspect some users would be unpleasantly surprised.

An alternative would be to add a new function with a different name (or multiple new functions), and eventually deprecate some or all of the old functions.

This could be a good time to clean up this area of API more holistically, because currently it is a little overcomplicated/inconsistent:

• fn find_handles<P: ProtocolPointer + ?Sized>() -> Result<Vec<Handle>>
• fn locate_handle_buffer(search_ty: SearchType<'_>) -> Result<HandleBuffer>
• fn locate_handle<'buf>(search_ty: SearchType<'_>, buffer: &'buf mut [MaybeUninit<Handle>]) -> Result<&'buf [Handle], Option<usize>>

find_handles is essentially the same as locate_handle_buffer(SearchType::ByProtocol(_)), except that it requires alloc because it returns a vec.

I think a reasonable simplification would be:

• Add three new functions: get_all_handles, find_handles_by_guid, and find_handles_by_register_notify. All of these new functions would require alloc and return a Vec<Handle>. They would have the new behavior of returning an empty Vec if no matching handles are found. Having three functions avoids needing to use SearchType for the very common operation of finding handles, which I've always found a bit annoying.
• Eventually deprecate find_handles and locate_handle_buffer. find_handles can be replaced by find_handles_by_guid(Protoco::GUID), and locate_handle_buffer can be replaced by one of the three new functions as long as alloc is enabled. (I am becoming skeptical that there is ever a reason not to have alloc enabled in UEFI projects. And locate_handle still exists for users that really want to avoid alloc.)
• Keep locate_handle unchanged. This is the "most raw" version that doesn't require alloc. A bit questionable whether this serves any real-world use; I imagine people always use the allocating functions, so churn here doesn't seem worth it to me. We can always revisit later.

[phip1611]

thanks for raising your concerns and pointing out some ideas, they are all valid! I like the idea of deprecating the old functions and provide new ones with new behavior!