dirent-util: Remove asserts on dirent64 == dirent

[kraj]

We already have assert_cc(_FILE_OFFSET_BITS == 64) which ensures that 64bit LFS functions are same as their original counterparts

Signed-off-by: Khem Raj raj.khem@gmail.com

[yuwata]

These lines are just for safety. Even if they are redundant, they should not hurt anything. Why do you want to drop them?

[kraj]

the lfs64 functions like dirent64 are guarded under -D_LARGEFILE64_SOURCE feature test macro which is indrectly defined with _GNU_SOURCE on glibc but not on musl. This make it work without -D_LARGEFILE64_SOURCE

[yuwata]

Hmm, but, the relation between _FILE_OFFSET_BITS == 64 vs the equivalence of dirent and dirent64 is not trivial.
So, I'd like to keep the current assertions.

This sounds mostly for musl. Then, I'd like to request to add -D_LARGEFILE64_SOURCE when build with musl.

[kraj]

Hmm, but, the relation between _FILE_OFFSET_BITS == 64 vs the equivalence of dirent and dirent64 is not trivial. So, I'd like to keep the current assertions.

Can you explain whats non-trivial ? In musl dirent64 is same as dirent when _LARGEFILE64_SOURCE is set. I see
glibc is similar. Maybe I am missing something

This sounds mostly for musl. Then, I'd like to request to add -D_LARGEFILE64_SOURCE when build with musl.

Sure, I have that already. This was just trying to clean up systemd.

[poettering]

Hmm, this is just safety stuff that doesnt do anything. It just checks that lfs support works the way it should. Sre you suggesting this breaks on musl? Or why the change?

[yuwata]

Hmm, but, the relation between _FILE_OFFSET_BITS == 64 vs the equivalence of dirent and dirent64 is not trivial. So, I'd like to keep the current assertions.

Can you explain whats non-trivial ? In musl dirent64 is same as dirent when _LARGEFILE64_SOURCE is set. I see glibc is similar. Maybe I am missing something

The difference between dirent and dirent64 is simple, but it is not simple when the guards __USE_FILE_OFFSET64 and __USE_LARGEFILE64 defined or set. People may set them or related guards explicitly in a wrong (at least, unexpected) way. Hence, I'd like to keep the conditions.

This sounds mostly for musl. Then, I'd like to request to add -D_LARGEFILE64_SOURCE when build with musl.

Sure, I have that already. This was just trying to clean up systemd.

So, I do not think this is a good cleanup... This potentially reduces safety.

[poettering]

Anyway, please explain what the rationale behind this is? Does this fix anything for you on musl? Or is this merely about cleaning up a test that is redundant in your eyes?

[poettering]

so, the reason why i think we should keep this extra safety net is mostly because _FILE_OFFSET_BITS is a generic cross-platform thing, and getdents() is a Linux-specific thing that various libcs are known to define differently even. Hence the assumption that the generic, widely-accepted concept also covers the Linux-specific function/structure here seems a bit too much of a stretch to me, hence the extra safety net.

[kraj]

it is one patch less to keep systemd working with latest musl. Since latest musl has deprecated LFS64 functions, this patch is required.

[poettering]

christ, musl doesn't care the least bit about compatibility, does it?

[poettering]

so does musl also get rid of getdents64() given the "64" in its name?

honestly it appears to me that musl builds should really just do #define dirent64 dirent locally, to maintain compat with glibc

[poettering]

@kraj btw, while i have you:

With 45519d1 you can drop https://git.openembedded.org/openembedded-core/tree/meta/recipes-core/systemd/systemd/0013-Define-glibc-compatible-basename-for-non-glibc-syste.patch

Also, current kernels have faccessat2(), hence https://git.openembedded.org/openembedded-core/tree/meta/recipes-core/systemd/systemd/0012-don-t-pass-AT_SYMLINK_NOFOLLOW-flag-to-faccessat.patch is actively breaking things on current kernels (and a terrible idea anyway)

Also this is entirely redundant: https://git.openembedded.org/openembedded-core/tree/meta/recipes-core/systemd/systemd/0002-Add-sys-stat.h-for-S_IFDIR.patch – since quite a while sys/stat.h is included already, just a few lines up

[keszybz]

I agree with @yuwata: this change increases risk. Ifdefs are easy to get wrong.

Maybe instead add a meson check if struct dirent64 is available and put #if !HAVE_STRUCT_DIRENT64 around this.

[yuwata]

Superseded by #27599.