tytso / e2fsprogs

Ext2/3/4 file system utilities
http://ext4.wiki.kernel.org
373 stars 219 forks source link

lib/ext2fs: llseek: simplify linux section #150

Closed floppym closed 4 months ago

floppym commented 1 year ago

This PR supersedes #144.

On 32-bit musl systems, off_t is always 8 bytes regardless of _FILE_OFFSET_BITS. The previous code did not cover this case.

The previous #ifdef logic was rather confusing, so I reworked it into a more understandable form.

ncopa commented 10 months ago

We hit this issue in alpine linux. Would be great to have this merged.

tytso commented 10 months ago

Have you tested to make sure this does the right thing on 32-bit platforms on other C libraries (including glibc and dietlibc)? The concern is always that we end up fixing things for muslc, but end up breaking on other C libraries ....

floppym commented 10 months ago

I can confirm that it works for 32-bit x86 on glibc and musl. I cannot test dietlibc.

floppym commented 10 months ago

I also tested with --enable-largefile and --disable-largefile.

With --enable-largefile, the lseek function is used for both glibc and musl.

With --disable-largefile, the lseek64 function is used on glibc, and lseek (with 64-bit off_t) is used on musl.

I suspect the llseek function would only be used on ancient versions of glibc that lack the lseek64 symbol. I doubt it would be used on any system still supported in the last 10 years. You might consider dropping this dead code.

thesamesam commented 10 months ago

Sent to ML as well at https://lore.kernel.org/linux-ext4/20231107233323.2013334-1-sam@gentoo.org/T/#mbf7b7ddd012030f76c81310672e747e7b5c82aae.

firasuke commented 5 months ago

Any updates to this?