_GNU_SOURCE Implies _LARGEFILE64_SOURCE in glibc, but in musl, the
default configuration doesn't enable _LARGEFILE64_SOURCE.

From include/dirent.h of musl, getdents64 is provided as getdents when
_LARGEFILE64_SOURCE is defined.

    #if defined(_LARGEFILE64_SOURCE)
    ...
    #define getdents64 getdents
    #endif

Let's define _LARGEFILE64_SOURCE to fix up this compile error:

    tools/testing/selftests/nolibc/nolibc-test.c: In function ‘test_getdents64’:
    tools/testing/selftests/nolibc/nolibc-test.c:453:8: warning: implicit declaration of function ‘getdents64’; did you mean ‘getdents’? [-Wimplicit-function-declaration]
      453 |  ret = getdents64(fd, (void *)buffer, sizeof(buffer));
          |        ^~~~~~~~~~
          |        getdents
    /usr/bin/ld: /tmp/ccKILm5u.o: in function `test_getdents64':
    nolibc-test.c:(.text+0xe3e): undefined reference to `getdents64'
    collect2: error: ld returned 1 exit status

Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index be7614c897b4..d9644244fc95 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
 #define _GNU_SOURCE
+#define _LARGEFILE64_SOURCE
 
 /* libc-specific include files
  * The program may be built in 3 ways:
-- 
2.25.1

On Fri, Jun 30, 2023, at 16:44, Zhangjin Wu wrote:
> _GNU_SOURCE Implies _LARGEFILE64_SOURCE in glibc, but in musl, the
> default configuration doesn't enable _LARGEFILE64_SOURCE.
>
> From include/dirent.h of musl, getdents64 is provided as getdents when
> _LARGEFILE64_SOURCE is defined.
>
>     #if defined(_LARGEFILE64_SOURCE)
>     ...
>     #define getdents64 getdents
>     #endif
>
> Let's define _LARGEFILE64_SOURCE to fix up this compile error:

I think a better solution would be to use the normal getdents() instead
of glibc getdents64(), but then define _FILE_OFFSET_BITS=64 to tell
glibc to use the modern version of all filesystem syscalls.

     Arnd

Hi, Arnd

> On Fri, Jun 30, 2023, at 16:44, Zhangjin Wu wrote:
> > _GNU_SOURCE Implies _LARGEFILE64_SOURCE in glibc, but in musl, the
> > default configuration doesn't enable _LARGEFILE64_SOURCE.
> >
> > From include/dirent.h of musl, getdents64 is provided as getdents when
> > _LARGEFILE64_SOURCE is defined.
> >
> >     #if defined(_LARGEFILE64_SOURCE)
> >     ...
> >     #define getdents64 getdents
> >     #endif
> >
> > Let's define _LARGEFILE64_SOURCE to fix up this compile error:
> 
> I think a better solution would be to use the normal getdents() instead
> of glibc getdents64(), but then define _FILE_OFFSET_BITS=64 to tell
> glibc to use the modern version of all filesystem syscalls.
> 

Just checked the getdents manpage[1] and the nolibc code, both of glibc and
nolibc don't provide the getdents() library routine but both of them provide
the getdents64(), only musl provide getdents() by default.

And as the manpage shows, it is not easy to call getdents() with glibc, we
need manually call syscall() and define the 'dirent' struct ourselves:

    glibc does not provide a wrapper for getdents(); call getdents()
    using syscall(2).  In that case you will need to define the
    linux_dirent or linux_dirent64 structure yourself.

And for nolibc, a getdents64() with linux_dirent64 struct (with int64_t offset)
is provided, there is either no getdents() currently.

This patch aims to let nolibc-test at least compile for musl and therefore we
can easily check the new tests for musl, glibc and nolibc together.

For the 64bit offset related stuff, we'd better delay it in another patchset
(part of full rv32 support), which will convert the off_t to 64bit by default.

Thanks,
Zhangjin

[1]: https://man7.org/linux/man-pages/man2/getdents.2.html

>      Arnd

On Fri, Jun 30, 2023, at 20:01, Zhangjin Wu wrote:
> Hi, Arnd
>
>> On Fri, Jun 30, 2023, at 16:44, Zhangjin Wu wrote:
>> > _GNU_SOURCE Implies _LARGEFILE64_SOURCE in glibc, but in musl, the
>> > default configuration doesn't enable _LARGEFILE64_SOURCE.
>> >
>> > From include/dirent.h of musl, getdents64 is provided as getdents when
>> > _LARGEFILE64_SOURCE is defined.
>> >
>> >     #if defined(_LARGEFILE64_SOURCE)
>> >     ...
>> >     #define getdents64 getdents
>> >     #endif
>> >
>> > Let's define _LARGEFILE64_SOURCE to fix up this compile error:
>> 
>> I think a better solution would be to use the normal getdents() instead
>> of glibc getdents64(), but then define _FILE_OFFSET_BITS=64 to tell
>> glibc to use the modern version of all filesystem syscalls.
>> 
>
> Just checked the getdents manpage[1] and the nolibc code, both of glibc and
> nolibc don't provide the getdents() library routine but both of them provide
> the getdents64(), only musl provide getdents() by default.
>
> And as the manpage shows, it is not easy to call getdents() with glibc, we
> need manually call syscall() and define the 'dirent' struct ourselves:
>
>     glibc does not provide a wrapper for getdents(); call getdents()
>     using syscall(2).  In that case you will need to define the
>     linux_dirent or linux_dirent64 structure yourself.
>
> And for nolibc, a getdents64() with linux_dirent64 struct (with int64_t offset)
> is provided, there is either no getdents() currently.
>
> This patch aims to let nolibc-test at least compile for musl and therefore we
> can easily check the new tests for musl, glibc and nolibc together.

Ok, I see. Your current approach should be fine then.

> For the 64bit offset related stuff, we'd better delay it in another patchset
> (part of full rv32 support), which will convert the off_t to 64bit by default.

Right, makes sense.


    ARnd