Re: [PATCH v4 2/4] rust: configfs: introduce rust support for configfs

[Daniel Almeida]

> 
>> 
>>> 
>>> [...]
>>> 
>>>>> +
>>>>> +/// # Safety
>>>>> +///
>>>>> +/// `this` must be a valid pointer.
>>>>> +///
>>>>> +/// If `this` does not represent the root group of a `configfs` subsystem,
>>>>> +/// `this` must be a pointer to a `bindings::config_group` embedded in a
>>>>> +/// `Group<Parent>`.
>>>>> +///
>>>>> +/// Otherwise, `this` must be a pointer to a `bindings::config_group` that
>>>>> +/// is embedded in a `bindings::configfs_subsystem` that is embedded in a
>>>>> +/// `Subsystem<Parent>`.
>>>>> +unsafe fn get_group_data<'a, Parent>(this: *mut bindings::config_group) -> &'a Parent {
>>>>> +    // SAFETY: `this` is a valid pointer.
>>>>> +    let is_root = unsafe { (*this).cg_subsys.is_null() };
>>>>> +
>>>>> +    if !is_root {
>>>>> +        // SAFETY: By C API contact, `this` is a pointer to a
>>>>> +        // `bindings::config_group` that we passed as a return value in from
>>>>> +        // `make_group`. Such a pointer is embedded within a `Group<Parent>`.
>>>> 
>>>> This phrase is confusing.
>>> 
>>> I am not sure how to rephrase it to be less confusing. The pointer is
>>> the thing returned from `make_group`. `make_group` would only return a
>>> pointer into a `Group<Parent>`.
>> 
>> The problem is this: "that we passed as a return value in from”, to pass something as a return value
>> is already hard to parse, and when you reach the “in from” part, it becomes even harder.
>> 
>> Just say a variation of what you said above, that is perfectly understandable.
>> 
>> What about:
>> 
>> ```
>> 
>> `this` is a pointer to a `bindings::config_group` that was returned from a call to `make_group`. The pointer is known
>> to be embedded within a `Group<Parent>`.
>> 
>> ```
> 
> How is this:
> 
>        // SAFETY: By C API contact,`this` was returned from a call to
>        // `make_group`. The pointer is known to be embedded within a
>        // `Group<Parent>`.
> 

This is good.

— Daniel