Re: [PATCH] [RFC] rust: error: Convert 0 being an error to Result

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Jan 11, 2024 at 08:29:06AM +0100, Behme Dirk (CM/ESO2) wrote:
> On 11.01.2024 07:59, Greg KH wrote:
> > On Thu, Jan 11, 2024 at 07:44:15AM +0100, Dirk Behme wrote:
> > > The existing to_result() takes a (signed) integer from a kernel C function
> > > and converts it to an error if it's negative. Additionally, there are
> > > kernel C functions returning an unsigned integer where 0 is the error case.
> > > For example gen_pool_alloc() and friends. Provide a mechanism to convert
> > > this to Result too.
> > > 
> > > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> > > ---
> > >   rust/kernel/error.rs | 10 ++++++++++
> > >   1 file changed, 10 insertions(+)
> > > 
> > > I'm unsure if something like this is acceptable. Therefore the RFC.
> > > But I want at least ask ;) In the end this is a slightly modified
> > > copy of the existing to_result().
> > > 
> > > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> > > index 376280b6a745a..3306b8f590866 100644
> > > --- a/rust/kernel/error.rs
> > > +++ b/rust/kernel/error.rs
> > > @@ -248,6 +248,16 @@ pub fn to_result(err: core::ffi::c_int) -> Result {
> > >       }
> > >   }
> > > +/// Converts an unsigned integer as returned by a C kernel function to EINVAL if it's zero,
> > > +/// and `Ok(u64)` otherwise.
> > > +pub fn to_result_zero(val: core::ffi::c_ulong) -> Result<u64> {
> > > +    if val == 0 {
> > > +        Err(code::EINVAL)
> > > +    } else {
> > > +        Ok(val)
> > > +    }
> > > +}
> > 
> > How would this be used?
> 
> One (randomly selected) usage example:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/mce/genpool.c#n107
> 
> node = (void *)gen_pool_alloc(mce_evt_pool, sizeof(*node));
> if (!node) {
> 	pr_warn_ratelimited("MCE records pool full!\n");
> 	return -ENOMEM;
> }
> 
> > 0 is normally not an error, why would you
> > need/want to turn that into an error value?

So you are treating NULL as 0 in rust bindings somehow?

I guess a usage of the new function would be good to see to clarify this
please.

thanks,

greg k-h