Re: [PATCH v3 2/2] rust: drop `error::to_result` and utilize `ToResult`

[Alice Ryhl <aliceryhl@google.com>]

On Mon, Oct 13, 2025 at 03:41:39PM +0300, Onur Özkan wrote:
> Current `to_result` helper takes a `c_int` and returns `Ok(())` on
> success and this has some issues like:
> 
>     - Callers lose the original return value and often have to store
> 	it in a temporary variable before calling `to_result`.
> 
>     - It only supports `c_int`, which makes callers to unnecessarily
> 	cast when working with other types (e.g. `u16` in phy
> 	abstractions). We even have some places that ignore to use
> 	`to_result` helper because the input doesn't fit in `c_int`
> 	(see [1]).
> 
> [1]: https://lore.kernel.org/all/20250822080252.773d6f54@nimda.home/
> 
> This patch removes the `error::to_result` function and replaces it
> with a more advanced helper, `ToResult` trait. This change brings
> three main benefits:
> 
>     - A better calling convention. Instead of wrapping function calls
> 	with `to_result`, we can now call `.to_result()` directly as an
> 	extension function.
> 
>     - The returned value is preserved as an unsigned integer on success
> 	which was previously discarded by error::to_result.
> 
>     - It's no longer limited with a single input type. E.g., right now we
> 	can call to_result on isize without manually casting them into i32.
> 
> So that the code that previously looked like:
> 
>     let ret = unsafe { bindings::some_ffi_call() };
>     to_result(ret).map(|()| SomeType::new(ret))
> 
> can now be written more directly as:
> 
>     unsafe { bindings::some_ffi_call() }.to_result()
> 	.map(SomeType::new)
> 
> Similarly, code such as:
> 
>     let res: isize = unsafe { bindings::some_ffi_call() };
>     if res < 0 {
> 	    return Err(Error::from_errno(res as i32));
>     }
> 
> can be done without manually casting into i32:
> 
>     unsafe { bindings::some_ffi_call() }.to_result()?;
> 
> This patch only fixes the callers that broke after changes on `to_result`.
> I haven't included all the improvements made possible by the new design
> since that could conflict with other ongoing patches [2]. Once this patch
> is approved and applied, I am planning to follow up with creating a
> "good first issue" on [3] for those additional changes.
> 
> [2]: https://lore.kernel.org/rust-for-linux/?q=to_result
> [3]: https://github.com/Rust-for-Linux/linux
> 
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089/topic/x/near/536374456
> Signed-off-by: Onur Özkan <work@onurozkan.dev>

I think we should keep the function call syntax and just change its
return type based on the new trait.

Or maybe even introduce a new method to keep the existing method that
returns Result<()>. Many functions only return 0 or an error, and I
don't want to map from int to () either.

Alice