Re: [RFC PATCH 05/19] rust: fs: introduce `INode<T>`

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

 



Alice Ryhl <alice@xxxxxxx> writes:

> On 10/18/23 14:25, Wedson Almeida Filho wrote:
>> +    /// Returns the super-block that owns the inode.
>> +    pub fn super_block(&self) -> &SuperBlock<T> {
>> +        // SAFETY: `i_sb` is immutable, and `self` is guaranteed to be valid by the existence of a
>> +        // shared reference (&self) to it.
>> +        unsafe { &*(*self.0.get()).i_sb.cast() }
>> +    }
>
> This makes me a bit nervous. I had to look up whether this field was a pointer
> to a superblock, or just a superblock embedded directly in `struct inode`. It
> does look like it's correct as-is, but I'd feel more confident about it if it
> doesn't use a cast to completely ignore the type going in to the pointer cast.
>
> Could you define a `from_raw` on `SuperBlock` and change this to:
>
>     unsafe { &*SuperBlock::from_raw((*self.0.get()).i_sb) }
>
> or perhaps add a type annotation like this:
>
>     let i_sb: *mut super_block = unsafe { (*self.0.get()).i_sb };
>     i_sb.cast()

I think it would also be nice to make the cast explicit:

  i_sb.cast::<SuperBlock<T>>()

otherwise the cast is no different than `as _` with all the caveats that
comes with.

BR Andreas




[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