[libc-commits] [PATCH] D147738: [libc] Enable linux directory entries syscalls in riscv64

[Mikhail Ramalho via Phabricator via libc-commits]

mikhail.ramalho added inline comments.


================
Comment at: libc/src/__support/File/dir.cpp:50
+  // returns a slightly different dirent struct:
+  struct linux_dirent {
+    unsigned long d_ino;
----------------
michaelrj wrote:
> this struct should be moved to its own file in `include/llvm-libc-types/` to match `struct dirent`
`struct linux_dirent` is kinda an oddball... glibc never defines it, see https://man7.org/linux/man-pages/man2/getdents.2.html:
`Note: There is no definition of struct linux_dirent in glibc; see NOTES.`

The notes mention: 
```
       Library support for getdents64() was added in glibc 2.30; Glibc
       does not provide a wrapper for getdents(); call getdents() (or
       getdents64() on earlier glibc versions) using syscall(2).  In
       that case you will need to define the linux_dirent or
       linux_dirent64 structure yourself.
```

Maybe I can add a comment just explaining that?

I also doesn't seem to be exposed by the kernel: https://elixir.bootlin.com/linux/v4.7/source/fs/readdir.c#L150


================
Comment at: libc/src/__support/File/dir.cpp:56
+#endif
+    char d_name[1];
+    /*
----------------
michaelrj wrote:
> you should probably add a comment clarifying that the 1 here represents the minimum width of this string, and that this struct has a variable length.
will do!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147738/new/

https://reviews.llvm.org/D147738