| Message ID | 20250304122119.21412-2-luis@igalia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix compilation errors when not using GNU libc | expand |
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 :) 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. 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) > { >
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. And thanks, Zorro, for your review! [1] https://www.gnu.org/software/libc/manual/html_node/Finding-Tokens-in-a-String.html Cheers,
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,
On Sat, Mar 08, 2025 at 11:21:19PM +0000, Luis Henriques wrote: > 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. Thanks Luis, breaking the building with musl is not a big issue, but please never break the current glibc build :) > > (But feel free to pick the second patch, as I believe it's still a valid > fix.) Today I'll make a fstests release, your second patch will be in the next release (after today). You can find it in patches-in-queue branch soon. Feel free to resend the whole patchset or resend this single one. Thanks, Zorro > > 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) > >>> { > >>> > >> > > >
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 #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) {
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(+)