-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use bindings for -D_FILE_OFFSET_BITS=64 #23
Conversation
d25a0bf
to
c8c4b72
Compare
Hm actually, on second though I'm not sure that this is a good idea. This would require that when linking against C code the I'll explore exposing the |
efb3b46
to
84f2f4a
Compare
Well, if you flip the scenario and C code does compile with Maybe it's a little cleaner to be very explicit that it's 64-bit, but then that makes cross platform development tricky again. "Which Honestly I don't think any FFI should ever expose any of these types, exactly because they are so fraught with idiosyncrasies like this. I would call it a mistake even for a pure C library to have these types in its interface. This should be business just between a program/library and libc/kernel only. Don't let the madness spread! :) But I fear I'm repeating things I've said already. Thanks for doing the actual work -- it looks like the new code organization has made this kind of type audit a lot easier. I'll try to give an actual code review tomorrow... |
A passing idea of compromise, in case I can't convince you fully hide these gory details -- how about an I suppose such an |
84f2f4a
to
70d567e
Compare
Hm true, that's a good point about libraries which are auto-compiled with this support. Although I think you're right in that those who are aware of this problem may actually avoid exposing I'm not too worried about selecting what the best stat option is on every platform, that's what the standard library is for! In all seriousness, though, libc is not intended to be cross platform at all, and instead is simply targeted at exposing what the underlying system provides. Along these lines I'd also be wary of defining our own abstractions here such as a Overall I'm leaning a bit more towards this as-is which exposes the 64-bit support as a separate set of functions and types. This does mean that users of the API will have to |
How about |
Oh right, that's a good point, I always seem to forget about those... I do agree that using |
Will you expose things like In general what you've added looks good to me. I think the b32/mod.rs Resource limits also have LFS versions, primarily to support 64-bit values in Technically Both There are other functions that could have LFS variants, but aren't represented in I noticed |
Thanks for looking this over! I think it's safe to say that we don't have to worry about the Windows story much here as I suspect that any project dealing with files on Windows falls into one of two categories:
Right now we do actually bind a silent translation of
Yeah so far it's verified for MIPS/ARM/x86, but if we add more platforms we can just break it apart as necessary.
Right! I remember when compiling with
Yeah I wasn't gonna worry too much about these, but seems harmless to go ahead and add.
Added! Elided Windows for now, however.
Cool, I'll hold off on these until the others are added anyway. By the way, is there a document you're reading these from? Would be good to list somewhere!
Ah yeah If you can't tell, most people don't use these bindings, they just use the standard library instead :) |
70d567e
to
e7fb8c7
Compare
I'm peering at the actual libc on Fedora like so:
You could look at I'm also referencing lintian's nice list of lfs-symbols. These are asserted not to be used by any 32-bit binaries, to be sure one is either explicitly using foo64 versions or implicitly getting there via
Which is good! So the job is mostly to provide that solid base for the standard library to use. :) |
Oy, musl doesn't like But I think this means a 32-bit musl target would go really badly right now... |
e7fb8c7
to
8c7497a
Compare
Yeah I have a feeling there's a few things that'll need to be fixed up for 32-bit MUSL, but we can always worry about all that later :) Let's see if this build is fixed... |
They're all added under the `foo64` names with the `foo64` types added as well, so they still need to be explicitly chosen.
8c7497a
to
7482522
Compare
Aha, yay! Got some green builds now. To confirm, this all sounds ok to you @cuviper? The major decision basically being to expose bindings as if you compiled C code with no flags rather than "what we think are the best set of C flags" |
It's not my preference, but I'll accept your decision. I guess we'll see in the standard library how annoying it is to Note, there are also other flags which may come up where "best" is more subjective, like |
Ok! In that case I'll merge, thanks @cuviper! |
Use bindings for -D_FILE_OFFSET_BITS=64
Tweak dependencies of Arch packages
# This is the 1st commit message: chore: add labels to FIXMEs # This is the commit message rust-lang#2: change label for .field FIXMEs # This is the commit message rust-lang#3: add fixme csv # This is the commit message rust-lang#4: Update fixmes.csv # This is the commit message rust-lang#5: Update fixmes.csv # This is the commit message rust-lang#6: Update fixmes.csv # This is the commit message rust-lang#7: Update fixmes.csv # This is the commit message rust-lang#8: Update fixmes.csv # This is the commit message rust-lang#9: Update fixmes.csv # This is the commit message rust-lang#10: Update fixmes.csv # This is the commit message rust-lang#11: Update fixmes.csv # This is the commit message rust-lang#12: Update fixmes.csv # This is the commit message rust-lang#13: Update fixmes.csv # This is the commit message rust-lang#14: Update fixmes.csv # This is the commit message rust-lang#15: Update fixmes.csv # This is the commit message rust-lang#16: Update fixmes.csv # This is the commit message rust-lang#17: Update fixmes.csv # This is the commit message rust-lang#18: updates to csv # This is the commit message rust-lang#19: Update fixmes.csv # This is the commit message rust-lang#20: Update fixmes.csv # This is the commit message rust-lang#21: Update fixmes.csv # This is the commit message rust-lang#22: Update fixmes.csv remove hurd" # This is the commit message rust-lang#23: Update fixmes.csv remove "'s
This should help "by default" dealing with large files on 32-bit Linux. Other platforms can opt in where necessary as well.