diff mbox series

[v3,03/14] selftests/nolibc: add _LARGEFILE64_SOURCE for musl

Message ID c61f64c4d1f69c19b41efae8fef5a94547089ca7.1688134399.git.falcon@tinylab.org (mailing list archive)
State Accepted
Commit 5f2de00e2c9ce00708bbf24f70ceda2ed35e9780
Headers show
Series selftests/nolibc: allow run with minimal kernel config | expand

Commit Message

Zhangjin Wu June 30, 2023, 2:44 p.m. UTC
_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(+)

Comments

Arnd Bergmann June 30, 2023, 3:23 p.m. UTC | #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
Zhangjin Wu June 30, 2023, 6:01 p.m. UTC | #2
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
Arnd Bergmann June 30, 2023, 6:45 p.m. UTC | #3
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
diff mbox series

Patch

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: