Re: [PATCH v5 1/4] rust: debugfs: Bind DebugFS directory creation

[Timur Tabi <ttabi@nvidia.com>]

On Mon, 2025-05-05 at 23:51 +0000, Matthew Maurer wrote:
> 
> +impl<'a> Entry<'a> {
> +    /// Constructs a new DebugFS [`Entry`] from the underlying pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The pointer must either be an error code, `NULL`, or represent a transfer of ownership of
> a
> +    /// live DebugFS directory. If this is a child directory or file, `'a` must be less than the
> +    /// lifetime of the parent directory.
> +    #[cfg(CONFIG_DEBUG_FS)]
> +    unsafe fn from_ptr(entry: *mut bindings::dentry) -> Self {
> +        Self {
> +            entry,
> +            _phantom: PhantomData,
> +        }
> +    }
> +
> +    #[cfg(not(CONFIG_DEBUG_FS))]
> +    fn new() -> Self {
> +        Self {
> +            _phantom: PhantomData,
> +        }
> +    }

I am new to Rust, so forgive me if this is a dumb question, but it looks to me that if
CONFIG_DEBUG_FS is defined, then you need to call from_ptr() to create a new Entry, but if
CONFIG_DEBUG_FS is not defined, then you need to call new() instead.  Is that right?  If so, is that
really idiomatic?

In the Dir implementation below, you are careful to call from_ptr() only from the CONFIG_DEBUG_FS
version of create(), and you call new() only from the !CONFIG_DEBUG_FS version of create().  So your
bases are covered as long as no driver tries to create an Entry from scratch. 

But I guess that can't happen because Entry is not public, right?

> +    /// Create a DebugFS subdirectory.
> +    ///
> +    /// Subdirectory handles cannot outlive the directory handle they were created from.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// let parent = Dir::new(c_str!("parent"));
> +    /// let child = parent.subdir(c_str!("child"));
> +    /// ```
> +    pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b> {
> +        Dir::create(name, Some(self))
> +    }
> +
> +    /// Create a new directory in DebugFS at the root.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// let debugfs = Dir::new(c_str!("parent"));
> +    /// ```
> +    pub fn new(name: &CStr) -> Self {
> +        Dir::create(name, None)
> +    }

Is there any real value to having two constructors, just to avoid passing None for the one time that
a root directory will be created?  The C code has no problem passing NULL.