Closed K900 closed 1 month ago
I'm not sure what the loader could do other than adding CFLAGS="-D_FILE_OFFSET_BITS=64"
to the build directly.
I'm not terribly familiar with the underlying system which causes the EOVERFLOW
. What would the readdir
code look like if it did detect the EOVERFLOW
errno? Just try again?
I can safely say that the loader is not suppose to be reading any files larger than 4gb (not sure what the file size limit is for the 32 bit readdir interface). Looking up FILE_OFFSET_BITS
, that seems to affect a lot of the file reading interfaces , but I can't say what would affect it enough to be a problem.
I can say that while 32 bit linux is supported by the Vulkan-Loader, I hope that it wont be in the future.
EOVERFLOW
isn't really handle-able here, but it should probably at least be reported so the user has any idea what's going on except "the loader says there are no files in the path where there are files".
The code would look something like
errno = 0; // reset errno
dir_entry = readdir(dir_stream);
if (errno != 0) {
// log this or something
} else if (NULL == dir_entry) {
break;
}
The issue here is also not the file size, it's the inode number - bcachefs (and XFS, and sometimes I think ZFS?) use 64-bit inode numbers, and without -D_FILE_OFFSET_BITS=64
glibc will try to fit them into a 32-bit value, fail, and that's what the EOVERFLOW
means.
I've recompiled my own copy of the loader with -D_FILE_OFFSET_BITS=64
and it seems OK, but I'm still not entirely sure there's no hidden 32-bit assumptions somewhere.
Also, I think the loader will have to support 32-bit applications for a long time...
Ah, right, issuing a more reasonable error message is better than "oops no drivers".
This feels like a design flaw with glibc, as trying to use 32 bit values for 64 bit values is never going to work.
I would be fine with a build change to always use 64 bit inodes, as you are absolutely right that support for 32 bit will continue for many years. No reason to arbitrarily limit support for no practical reason.
It's a backwards compatibility thing, a lot of older code was written with 32-bit inodes in mind, and C is not typed enough to actually prevent misuse of this at build time. Looking at the code more, I don't see any uses of d_ino
anywhere so it shooooooooould be fine?
There's really two issues here:
readdir
is not checked for errno, which would surface issue 2EOVERFLOW
as soon as it hits a big enough inode number and then just silently ignores the rest of the files in the directory.Just building the loader with
CFLAGS="-D_FILE_OFFSET_BITS=64"
seems to make it work at a glance, but I'm not familiar enough with the codebase to tell if something else will explode if I do that.