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

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/include/llvm-libc-types/struct_linux_dirent.h:1
+//===-- Definition of type struct linux dirent ----------------------------===//
+//
----------------
sivachandra wrote:
> michaelrj wrote:
> > mikhail.ramalho wrote:
> > > sivachandra wrote:
> > > > This is an internal type so should not be define in a public header file.
> > > This sounds contradictory to what @michaelrj suggested in one of his comments:
> > > 
> > > `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.`
> > From what I can tell from the man page, the `linux_dirent` struct is somewhat standard, but also often not defined by the C library. Given that, I personally prefer defining it like the other `dirent` struct as a public type. If we decide not to define it publicly, I think we should still define it in a separate header and use `ifdef` to include it as needed. That header might end up in `__support/File/linux`.
> Sorry, I missed @michaelrj's comment - AFAICT, `linux_dirent` is only relevant for the syscall, so we should not expose it publicly. We have examples of other such types in the libc: https://github.com/llvm/llvm-project/blob/main/libc/src/signal/linux/signal_utils.h#L29. And since it would be an internal type, you should name it `LinuxDirent`.
> From what I can tell from the man page, the `linux_dirent` struct is somewhat standard, but also often not defined by the C library. 

You are perhaps looking at https://man7.org/linux/man-pages/man2/getdents.2.html, which says:

```
These are not the interfaces you are interested in.  Look at readdir(3) for the POSIX-conforming C library interface.  This page documents the bare kernel system call interfaces.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147738