Use linux_dirent64 type with sys_getdents64#1207
Conversation
|
In the man page of getdents, it says:
I think this implies that the availability of getdents64 depends only on the kernel version (greater than 2.4). It doesn't depend on whether the CPU architecture is 32-bit or 64-bit. If we can test it on a 32-bit machine, we can sys_getdents64 everywhere. |
|
@xuyao0127 The original Linux getdents() system call did not handle large There's no need to test on a 32-bit architecture. The system call will work on both, and it is intended simply for large filesystems, which can be accessed by any Linux architecture. RISC-V and ARM64 implementations of the Linux kernel omitted getdents because getdents64 is a superset of those capabilities. So, there's never any need to call getdents. We don't care if glibc on 32-bit x86 chose to use getdents or getdents64. We only need to know that the 32-bit Linux kernel does support getdents64. |
|
@gc00 I have updated the code to use getdents64 everywhere. |
There was a problem hiding this comment.
Pull Request Overview
This PR updates directory-reading logic to use the 64-bit getdents64 syscall and corresponding linux_dirent64 structure on 64-bit platforms.
- Replace uses of
SYS_getdentsandlinux_direntwithSYS_getdents64andlinux_dirent64 - Remove legacy
linux_direntdefinition from the header - Introduce
linux_dirent64in the implementation
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| jalib/jfilesystem.h | Removed old linux_dirent struct; no 64-bit struct defined here |
| jalib/jfilesystem.cpp | Added linux_dirent64, switched syscall invocation to SYS_getdents64 |
Comments suppressed due to low confidence (2)
jalib/jfilesystem.h:29
- The header still needs a public definition of
linux_dirent64so consumers of the filesystem API can compile against it; consider moving the struct definition from the .cpp into this header undernamespace jalib.
- struct linux_dirent {
jalib/jfilesystem.cpp:36
- The code uses
SYS_getdents64but there’s no inclusion of<sys/syscall.h>, which may lead to undefinedSYS_getdents64; add the proper header so the syscall constant is defined.
struct linux_dirent64 {
| @@ -33,14 +33,13 @@ | |||
| #include <sys/utsname.h> | |||
| #include <unistd.h> | |||
|
|
|||
There was a problem hiding this comment.
Defining linux_dirent64 in the global namespace may conflict with other symbols; wrap this struct in the jalib (or jalib::Filesystem) namespace to mirror the header’s organization.
| namespace jalib { | |
| namespace Filesystem { |
|
|
||
| while (true) { | ||
| int nread = jalib::syscall(SYS_getdents, fd, buf, allocation); | ||
| int nread = jalib::syscall(SYS_getdents64, fd, buf, allocation); |
There was a problem hiding this comment.
Using SYS_getdents64 unconditionally may break on non-64-bit architectures; consider guarding this call with an #if check for supported ABIs or providing a fallback to SYS_getdents where appropriate.
Use sys_getdents64 and linux_dirent64 on 64-bit machines.