Re: xfsdump musl patch questions

[Donald Douwsma <ddouwsma@redhat.com>]

On 31/8/25 04:39, Holger Hoffstätte wrote:
> On 2025-08-30 20:10, Adam Thiede wrote:
> 
>> Thanks - using -D_LARGEFILE64_SOURCE also fixes the issue without the 
>> enormous patch. However the following small patch is necessary since 
>> alpine builds with -Wimplicit-function-declaration
>>
>> diff --git a/invutil/invidx.c b/invutil/invidx.c
>> index 5874e8d..9506172 100644
>> --- a/invutil/invidx.c
>> +++ b/invutil/invidx.c
>> @@ -28,6 +28,7 @@
>>   #include <sys/stat.h>
>>   #include <string.h>
>>   #include <uuid/uuid.h>
>> +#include <libgen.h>
>>
>>   #include "types.h"
>>   #include "mlog.h"
>> diff --git a/common/main.c b/common/main.c
>> index 6141ffb..f5e959f 100644
>> --- a/common/main.c
>> +++ b/common/main.c
>> @@ -38,6 +38,7 @@
>>   #include <string.h>
>>   #include <uuid/uuid.h>
>>   #include <locale.h>
>> +#include <libgen.h>
>>
>>   #include "config.h"
> 
> aargh..basename again!
> 
> We fix it like this:
> https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-fs/xfsdump/files/xfsdump-3.1.12-mimic-basename-for-nonglibc.patch
> 
> It's trickier than it looks since you need to actually audit the
> call sites, else you might get crashes at runtime.

My initial feeling is to just include libgen.h in include/config.h.in as

#if !defined(__GLIBC__)
#include <libgen.h>
#endif

Its less targeted than your patch which only touches files using
basename, but xfsprogs resolved this by including libgen.h in
platform_defs.in unconditionally somewhere after string.h

>> I think this one would be good to include at the very least.
> 
> The thing is that your longer patch is conceptually the better way
> forward, just a bit incomplete as it is. We just discussed this on
> IRC and I can quote what our resident toolchain guru Sam had to say,
> which provides some background as well:
> 
> [19:52:50] <sam_> the patch is wrong as it is, at least without some 
> rationale & checking that AC_SYS_LARGEFILE is used (to guarantee off_t 
> is 64-bit on 32-bit glibc systems)
> [19:53:05] <sam_> in general, the stat64, off64_t, etc types should go 
> away (they're transitional)
> [19:53:12] <sam_> the issue is when getting rid of them, people often 
> get it wrong
> [19:53:25] <sam_> the types were added into glibc to allow people to 
> have dual ABIs in their headers
> [19:53:26] <sam_> it never took off
> [19:53:45] <sam_> you can look at the thread where I ported xfsprogs itself
> 
> The thread he's referring to starts here:
> https://lore.kernel.org/linux-xfs/20240205232343.2162947-1-sam@gentoo.org/

Thanks for the context, I noticed the comment about _LARGEFILE64_SOURCE
in the release notes you mentioned earlier
https://musl.libc.org/releases.html

In general xfsdump should follow how xfsprogs builds.

Ill also look over the other changes made to the xfsprogs build and
see if there's anything else that's been missed.

Don