Fix for #30876
Fix BootServices.locateHandleLen() #30877
jsentity/zig:master into master @ -702,8 +702,9 @@ pub const BootServices = extern struct {
&len,
null,
)){
.success=>return@divExact(len,@sizeOf(Handle)),
I'd prefer to keep an explicit switch prong handling .success with unreachable and a comment explaining why.
alright, added the changes. Would it also make sense to create narrower errors for invalid_parameter, since most cases (if not all, currently) are unreachable?
Indeed,. according to the spec EFI_INVALID_PARAMETER only occurs when NoHandles or Buffer are null - which they aren't.
in !30877 (commit 72f1996084) I also handled invalid_parameter with unreachable, but later removed it, since I wasn't sure if the spec could add more reasons in the future for returning that error, in which case, it could become reachable
I think for std.posix (or its replacements now that it's getting dissolved) we're also using unreachable for kernel return codes liberally, it can be updated for a new version if needed.
Personally, I think unreachable is better suited for situations where your code can guarantee an invariant internally. In this case, and for (std.posix), the invariant depends on an external factor that can change at any time. Therefore, I believe error.Unexpected is more appropriate in this case, since existing code using older Zig releases in ReleaseFast/ReleaseSmall could reach UB if the external factor changes. So all zig projects reliant on BootServices.locateHandleLen() would need the newest version, otherwise a patch for every older release would be required, unless every project not using the newest version would essentially be left unsupported and wouldn't be able to rely on the std for avoiding UB. The same issue happened to me recently in 0.15.2 with posix.read (https://github.com/ziglang/zig/issues/25344), where I was forced to create a custom implementation for it, since I was unable to catch the error during debug builds, and would invite UB during unchecked releases if I relied on the std for the functionality.
Okay, let's fold it into the else prong then given that it's not an expected error.
okay so then .invalid_parameter cascades into else and the branch is commented out, explaining why it is considered unreachable/unexpected.
Should the same happen to .success or do we leave it as is?
I suppose the same should happen to .success, yes. I'll also have to retract the std.posix argument given https://github.com/ziglang/zig/issues/6389 is accepted.
d1ba05e998
b43f62182d
Thanks!
(cancelled auto-merge as per above discussion)
alright should be good to go now
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?