On Mon Sep 29, 2025 at 3:06 AM CEST, Alistair Popple wrote: > On 2025-09-26 at 12:20 +1000, Alexandre Courbot <[email protected]> wrote... >> On Thu Sep 25, 2025 at 3:32 PM JST, Alistair Popple wrote: >> <snip> >> >> > + #[expect(unused)] >> >> > + pub(crate) fn receive_msg_from_gsp<M: GspMessageFromGsp, R>( >> >> > + &mut self, >> >> > + timeout: Delta, >> >> > + init: impl FnOnce(&M, SBuffer<core::array::IntoIter<&[u8], >> >> > 2>>) -> Result<R>, >> >> > + ) -> Result<R> { >> >> > + let (driver_area, msg_header, slice_1) = wait_on(timeout, || { >> >> > + let driver_area = self.gsp_mem.driver_read_area(); >> >> > + // TODO: find an alternative to as_flattened() >> >> > + #[allow(clippy::incompatible_msrv)] >> >> > + let (msg_header_slice, slice_1) = driver_area >> >> > + .0 >> >> > + .as_flattened() >> >> > + .split_at(size_of::<GspMsgElement>()); >> >> > + >> >> > + // Can't fail because msg_slice will always be >> >> > + // size_of::<GspMsgElement>() bytes long by the above >> >> > split. >> >> > + let msg_header = >> >> > GspMsgElement::from_bytes(msg_header_slice).unwrap(); >> >> >> >> Any reason we're not just using unwrap_unchecked() here then? >> > >> > Because whilst my assertions about the code are currently correct if it >> > ever >> > changes I figured it would be better to explicitly panic than end up with >> > undefined behaviour. Is there some other advantage to using >> > unwrap_unchecked()? >> > I can't imagine there'd be much of a performance difference. >> >> Here I think we should just use the `?` operator. The function already >> returns a `Result` so it would fit. > > Actually note quite true - this is in a closure that must return `Option<_>` > so returning `Result` doesn't fit. However it still fits because I just > noticed > `::from_bytes()` returns an `Option` so `?` will still work.
More in general, as by now, unwrap() panics the kernel, which is an absolute last resort, that should only be considered if there's really no other option. Please also see [1] and [2]. [1] https://docs.kernel.org/process/coding-style.html#do-not-crash-the-kernel [2] https://github.com/Rust-for-Linux/linux/issues/1191
