Re: [PATCH 3/5] rust: add support for get_stats64 in struct net_device_ops

[Boqun Feng <boqun.feng@gmail.com>]

On Mon, Jun 05, 2023 at 01:57:36PM +0000, FUJITA Tomonori wrote:
> Hi,
> 
> On Sun,  4 Jun 2023 12:48:29 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
> 
> > FUJITA Tomonori writes:
> >> +/// Corresponds to the kernel's `struct rtnl_link_stats64`.
> >> +pub struct RtnlLinkStats64(*mut bindings::rtnl_link_stats64);
> >> +
> >> +impl RtnlLinkStats64 {
> >> +    /// Updates TX stats.
> >> +    pub fn set_tx_stats(&mut self, packets: u64, bytes: u64, errors: u64, dropped: u64) {
> >> +        // SAFETY: The safety requirement ensures that the pointer is valid.
> >> +        unsafe {
> >> +            (*self.0).tx_packets = packets;
> >> +            (*self.0).tx_bytes = bytes;
> >> +            (*self.0).tx_errors = errors;
> >> +            (*self.0).tx_dropped = dropped;
> >> +        }
> >> +    }
> >> +}
> > 
> > It seems like this type is always used behind a reference. Your current
> > approach has the disadvantage that you're introducing an extra layer of
> > indirection. You can avoid that like this:
> > 
> > ```
> > /// Corresponds to the kernel's `struct rtnl_link_stats64`.
> > #[repr(transparent)]
> > pub struct RtnlLinkStats64(Opaque<bindings::rtnl_link_stats64>);
> > 
> > impl RtnlLinkStats64 {
> >     /// # Safety
> >     ///
> >     /// For the duration of the lifetime 'a, the pointer must be valid for writing and nobody else
> >     /// may read or write to the `rtnl_link_stats64` object.
> >     pub unsafe fn from_raw<'a>(ptr: *mut bindings::rtnl_link_stats64) -> &'a mut Self {
> >     	unsafe { &mut *(ptr as *mut Self) }
> >     }
> 
> I was wondering which I should use here, a pointer or Opaque. Can you
> give guidelines on which to use?
> 

Always avoid raw pointers in data structures, unless there is no
dereference on the pointers (i.e. use the pointer types as plain data).

The reason is as Alice mentioned above.

Regards,
Boqun

> The condition in the above commment is met, Opaque should be used?