Re: [RFC PATCH 04/19] rust: fs: introduce `FileSystem::super_params`

[Alice Ryhl <alice@ryhl.io>]

On 10/18/23 18:34, Benno Lossin wrote:>> +        from_result(|| {
>> +            // SAFETY: The C callback API guarantees that `fc_ptr` is valid.
>> +            let fc = unsafe { &mut *fc_ptr };
> 
> This safety comment is not enough, the pointer needs to be unique and
> pointing to a valid value for this to be ok. I would recommend to do
> this instead:
> 
>      unsafe { addr_of_mut!((*fc_ptr).ops).write(&Tables::<T>::CONTEXT) };

It doesn't really need to be unique. Or at least, that wording gives the 
wrong intuition even if it's technically correct when you use the right 
definition of "unique".

To clarify what I mean: Using `ptr::write` on a raw pointer is valid if 
and only if creating a mutable reference and using that to write is 
valid. (Assuming the type has no destructor.)

Of course, in this case you *also* have the difference of whether you 
create a mutable to the entire struct or just the field.
>> +                // SAFETY: This is a newly-created inode. No other references to it exist, so it is
>> +                // safe to mutably dereference it.
>> +                let inode = unsafe { &mut *inode };
> 
> The inode also needs to be initialized and have valid values as its fields.
> Not sure if this is kept and it would probably be better to keep using raw
> pointers here.

My understanding is that this is just a safety invariant, and not a 
validity invariant, so as long as the uninitialized memory is not read, 
it's fine.

See e.g.:
https://github.com/rust-lang/unsafe-code-guidelines/issues/346

Alice