Re: [PATCH v8 2/2] rust: xarray: Add an abstraction for XArray

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 21, 2024 at 05:22:11PM -0700, John Hubbard wrote:
> On 3/19/24 2:32 AM, Philipp Stanner wrote:
> ...
> > > 
> > > > +        if ret < 0 {
> > > > +            Err(Error::from_errno(ret))
> > > > +        } else {
> > > > +            guard.dismiss();
> > > > +            Ok(id as usize)
> > > > +        }
> > > 
> > > You could make this easier to read using to_result.
> > > 
> > > to_result(ret)?;
> > > guard.dismiss();
> > > Ok(id as usize)
> > 
> > My 2 cents, I'd go for classic kernel style:
> > 
> > 
> > if ret < 0 {
> >      return Err(...);
> > }
> > 
> > guard.dismiss();
> > Ok(id as usize)
> > 
> 
> Yes.
> 
> As a "standard" C-based kernel person who is trying to move into Rust
> for Linux, I hereby invoke my Rust-newbie powers to confirm that the
> "clasic kernel style" above goes into my head much faster and easier,
> yes. :)
> 

Hope I'm still belong to a "standard C-based kernel person" ;-) My
problem on "if ret < 0 { ... }" is: what's the type of "ret", and is it
"negative means failure" or "non-zero means failure"? Now for this
particular code, the assignment of "ret" and the use of "ret" is pretty
close, so it doesn't matter. But in the code where "ret" is used for
multiple function calls and there is code in-between, then I'd prefer we
use `to_result` (i.e. `Result` type and question mark operator).

> I hope I'm not violating any "this is how Rust idioms must be"
> conventions. But if not, then all other things being equal, it is of
> course a nice touch to make the code more readable to the rest of the
> kernel folks.
> 

One more extra point from myself only: if one is using Rust for drivers
or subsystem they are going to take care of it in the future, it's
totally fine for them to pick coding styles that they feel comfortable,
I don't want to make people who do the real work feel frustrated because
"this is how Rust idioms must be", also I don't believe tools should
restrict people. But in the "kernel" crate (i.e. for core kernel part),
I want to make it "Rusty" since it's the point of the experiement ("you
asked for it ;-)).

Hope we can find a balanced point when we learn more and more ;-)

Regards,
Boqun

> 
> thanks,
> -- 
> John Hubbard
> NVIDIA
> 




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux