Re: [PATCH 1/2] src: include libgen.h for function basename() if not using GNU libc

[Luis Henriques <luis@igalia.com>]

Hi Zorro,

On Tue, Mar 04 2025, Luis Henriques wrote:

> On Tue, Mar 04 2025, Zorro Lang wrote:
>
>> On Tue, Mar 04, 2025 at 12:21:18PM +0000, Luis Henriques wrote:
>>> Compiling fstests on a system without the GNU libc fails because the
>>> basename() is defined in libgen.h by POSIX and not in string.h.
>>> 
>>> Fix that by including the libgen.h header if __USE_GNU isn't defined.
>>> 
>>> Signed-off-by: Luis Henriques <luis@igalia.com>
>>> ---
>>>  src/aio-dio-regress/aio-dio-append-write-read-race.c | 3 +++
>>>  src/splice-test.c                                    | 3 +++
>>>  src/t_ext4_dax_inline_corruption.c                   | 3 +++
>>>  src/t_ext4_dax_journal_corruption.c                  | 3 +++
>>>  src/t_mmap_collision.c                               | 3 +++
>>>  src/t_mmap_dio.c                                     | 3 +++
>>>  6 files changed, 18 insertions(+)
>>> 
>>> diff --git a/src/aio-dio-regress/aio-dio-append-write-read-race.c b/src/aio-dio-regress/aio-dio-append-write-read-race.c
>>> index d9f8982f007c..1a0c8e99c61f 100644
>>> --- a/src/aio-dio-regress/aio-dio-append-write-read-race.c
>>> +++ b/src/aio-dio-regress/aio-dio-append-write-read-race.c
>>> @@ -19,6 +19,9 @@
>>>  #include <sys/stat.h>
>>>  #include <fcntl.h>
>>>  #include <errno.h>
>>> +#ifndef __USE_GNU
>>> +#include <libgen.h>
>>> +#endif
>>
>> Do you use clang and musl to build fstests? I'm even not sure if fstests
>> can be built with them :) 
>
> Yes, I'm able to compile and run fstests using gcc (not clang) and musl.
> Though there's a hack I still need to use to compile, which is to define
> CFLAGS="-D_LARGEFILE64_SOURCE".  Since that doesn't require changing
> anything, I haven't yet investigated how that could be fixed.
>
>> I don't have experience on this part, but I don't mind letting fstests
>> to be supported by other libc. The patch looks safe for current users.
>>
>> If anyone has this experience, please feel free to review. I'll think
>> about merging this patch if no objection from others.
>
> The only links I can provide you with some extra info are [1] (section
> about basename) and the basename(3) man page.  Looking at it again makes
> me think that including libgen.h unconditionally should also be OK,
> forcing the POSIX implementation to be used instead.  But using this
> #ifndef seems to be used in other projects.
>

Please hold on this patch as I think some of the changes are broken.  The
POSIX basename() is a bit different from the GNU version, and needs to be
handled with a bit more care.  I'll resend this later in a few days.

(But feel free to pick the second patch, as I believe it's still a valid
fix.)

Cheers,
-- 
Luís

> And thanks, Zorro, for your review!
>
> [1] https://www.gnu.org/software/libc/manual/html_node/Finding-Tokens-in-a-String.html
>
> Cheers,
> -- 
> Luís
>
>>
>> Thanks,
>> Zorro
>>
>>>  
>>>  #include <libaio.h>
>>>  #include <pthread.h>
>>> diff --git a/src/splice-test.c b/src/splice-test.c
>>> index eb8636738064..e6753a15dc47 100644
>>> --- a/src/splice-test.c
>>> +++ b/src/splice-test.c
>>> @@ -18,6 +18,9 @@
>>>  #include <string.h>
>>>  #include <errno.h>
>>>  #include <malloc.h>
>>> +#ifndef __USE_GNU
>>> +#include <libgen.h>
>>> +#endif
>>>  
>>>  unsigned int sector_size;
>>>  unsigned int buffer_size;
>>> diff --git a/src/t_ext4_dax_inline_corruption.c b/src/t_ext4_dax_inline_corruption.c
>>> index e1a39a6c1e46..a6cb768512fc 100644
>>> --- a/src/t_ext4_dax_inline_corruption.c
>>> +++ b/src/t_ext4_dax_inline_corruption.c
>>> @@ -10,6 +10,9 @@
>>>  #include <sys/types.h>
>>>  #include <time.h>
>>>  #include <unistd.h>
>>> +#ifndef __USE_GNU
>>> +#include <libgen.h>
>>> +#endif
>>>  
>>>  #define PAGE(a) ((a)*0x1000)
>>>  #define STRLEN 256
>>> diff --git a/src/t_ext4_dax_journal_corruption.c b/src/t_ext4_dax_journal_corruption.c
>>> index ba7a96e43559..4e5762d77058 100644
>>> --- a/src/t_ext4_dax_journal_corruption.c
>>> +++ b/src/t_ext4_dax_journal_corruption.c
>>> @@ -10,6 +10,9 @@
>>>  #include <sys/types.h>
>>>  #include <time.h>
>>>  #include <unistd.h>
>>> +#ifndef __USE_GNU
>>> +#include <libgen.h>
>>> +#endif
>>>  
>>>  #define PAGE(a) ((a)*0x1000)
>>>  #define STRLEN 256
>>> diff --git a/src/t_mmap_collision.c b/src/t_mmap_collision.c
>>> index c872f4e26940..638424d2e619 100644
>>> --- a/src/t_mmap_collision.c
>>> +++ b/src/t_mmap_collision.c
>>> @@ -24,6 +24,9 @@
>>>  #include <sys/stat.h>
>>>  #include <sys/types.h>
>>>  #include <unistd.h>
>>> +#ifndef __USE_GNU
>>> +#include <libgen.h>
>>> +#endif
>>>  
>>>  void *dax_data;
>>>  int nodax_fd;
>>> diff --git a/src/t_mmap_dio.c b/src/t_mmap_dio.c
>>> index 6c8ca1a39181..342e2c6deb4b 100644
>>> --- a/src/t_mmap_dio.c
>>> +++ b/src/t_mmap_dio.c
>>> @@ -14,6 +14,9 @@
>>>  #include <libaio.h>
>>>  #include <errno.h>
>>>  #include <sys/time.h>
>>> +#ifndef __USE_GNU
>>> +#include <libgen.h>
>>> +#endif
>>>  
>>>  void usage(char *prog)
>>>  {
>>> 
>>
>