dirent.h: Add missing field d_ino

[no1wudi]

Summary

Currently implementations of dirent.h do not have a field for the inode number, but it is required by POSIX.

For better compatibility with POSIX based applications, it is should be added even if it is not actually implemented.

PR coupled with nuttx-apps PR apache/nuttx-apps#2593.

Impact

• include/dirent.h.
• include/nuttx/fs/hostfs.h.
• Makes implementation compatible with POSIX.

Testing

CI and local machine

[yamt]

if we add the field, i guess we should set a sane value.
at least it should be consistent with st_ino.

[no1wudi]

if we add the field, i guess we should set a sane value. at least it should be consistent with st_ino.

I'm not familiar with implementations of VFS, did you mean something like this?

static ssize_t dir_read(FAR struct file *filep, FAR char *buffer,
                        size_t buflen)
{
  FAR struct fs_dirent_s *dir = filep->f_priv;
#ifndef CONFIG_DISABLE_MOUNTPOINT
  FAR struct inode *inode = dir->fd_root;
#endif
  FAR struct dirent *dirent = (FAR struct dirent *)buffer;
  int ret;

  /* Verify that we were provided with a valid directory structure */

  if (buffer == NULL || buflen < sizeof(struct dirent))
    {
      return -EINVAL;
    }

  /* The way we handle the readdir depends on the type of inode
   * that we are dealing with.
   */

#ifndef CONFIG_DISABLE_MOUNTPOINT
  if (INODE_IS_MOUNTPT(inode))
    {
      ret = inode->u.i_mops->readdir(inode, dir, dirent);
    }
  else
#endif
    {
      /* The node is part of the root pseudo file system */

      ret = read_pseudodir(dir, dirent);
    }

  /* ret < 0 is an error. Special case: ret = -ENOENT is end of file */

  if (ret < 0)
    {
      if (ret == -ENOENT)
        {
          ret = 0;
        }

      return ret;
    }

  >>> New added  here <<<

  dirent->d_ino = dir->fd_root->i_ino;


  filep->f_pos++;
  return sizeof(struct dirent);
}

[yamt]

d_ino should match st_ino of the file referenced by the directory entry.

a naive implementation would be something like:

fstat(dirent->d_name, &st);
dirent->d_ino = st.st_ino;

iirc, on nuttx, most filesystems just use st_ino = 0.
for those filesystems, it can be optimized to d_ino = 0.

[yamt]

honestly speaking, i'm not sure if it's worth to implement d_ino for nuttx.
as the most filesystems here don't implement inode numbers at all,
applications using d_ino likely need modifications to deal with the situation anyway.

what's your motivation to add d_ino?

[no1wudi]

what's your motivation to add d_ino?

@yamt I'm porting Rust std library to NuttX, the ino is required by it: https://doc.rust-lang.org/std/os/unix/fs/trait.DirEntryExt.html#tymethod.ino

But this is not used by Rust itself, leave it to 0 is OK if nobody really need it. So in the first PR I just add the missing field to make the compiler happy.

[no1wudi]

And I'm reusing the unix port, so the strcut layout should be aligned to POSIX if possible even if we don't implement it yet

[lupyuen]

And I'm reusing the unix port, so the strcut layout should be aligned to POSIX if possible even if we don't implement it yet

@no1wudi I think we might need to customise the Unix Port, and make it work for NuttX? It seems a bit odd that we need to add extra Kernel Fields to NuttX, just to support Rust.

[no1wudi]

@no1wudi I think we might need to customise the Unix Port, and make it work for NuttX? It seems a bit odd that we need to add extra Kernel Fields to NuttX, just to support Rust.

It's OK that just leave it to 0 in Rust side for NuttX, if it's better

[yamt]

what's your motivation to add d_ino?

@yamt I'm porting Rust std library to NuttX, the ino is required by it: https://doc.rust-lang.org/std/os/unix/fs/trait.DirEntryExt.html#tymethod.ino

But this is not used by Rust itself, leave it to 0 is OK if nobody really need it. So in the first PR I just add the missing field to make the compiler happy.

as it can be a bit expensive to perform fstat equivalent on each entries, it might be better to avoid doing that by default.

however, unfortunately, there is no reserved values for inode numbers to indicate the "no inode number available" situation as far as i know.
that is, 0 is not really a special value.
it's unfortunate because, even on a posix environment, it's often tricky to provide inode numbers for some filesystems w/o hard links.
having said that, my impression is that it isn't too uncommon to use 0 for that purpose. (at least it's certainly better than uninitialized stack garbage.)

wrt rust, depending on how serious your port is, you might need to consider emulating the inode number by yourself.
(not only d_ino, but also st_ino.)
otoh, maybe the least invasive thing is to patch your rust port to provide 0 by itself, rather than modifying nuttx.

FYI, wasi-libc tries to emulate d_ino by itself for similar reasons. (well, another motivation there is windows support)
https://github.com/WebAssembly/wasi-libc/blob/7d4d3b83fc66c79b3faa5989e67ed2d1042dacaf/libc-bottom-half/cloudlibc/src/libc/dirent/readdir.c#L87-L110

[no1wudi]

wrt rust, depending on how serious your port is, you might need to consider emulating the inode number by yourself. (not only d_ino, but also st_ino.) otoh, maybe the least invasive thing is to patch your rust port to provide 0 by itself, rather than modifying nuttx.

Maybe not 0 but panic if user try to use it for now on NuttX, Rust std allow some unsupported features if platform don't support it.

FYI, wasi-libc tries to emulate d_ino by itself for similar reasons. (well, another motivation there is windows support) https://github.com/WebAssembly/wasi-libc/blob/7d4d3b83fc66c79b3faa5989e67ed2d1042dacaf/libc-bottom-half/cloudlibc/src/libc/dirent/readdir.c#L87-L110

Thanks for your information, I guess it should be emulated in NuttX side as your comments above instead of Rust side.