[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:
> mikhail.ramalho wrote:
> > 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
> glibc not providing the struct publicly doesn't mean that we can't. I think it's cleaner to have the type defined in `llvm-libc-types` and then use `#ifdef` to include the appropriate struct.
got it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147738