Skip to content
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

s390x-musl: define O_LARGEFILE constant #3262

Merged
merged 1 commit into from Jun 4, 2023

Conversation

nekopsykose
Copy link
Contributor

@rustbot
Copy link
Collaborator

rustbot commented Jun 1, 2023

r? @JohnTitor

(rustbot has picked a reviewer for you, use r? to override)

@bossmc
Copy link
Contributor

bossmc commented Jun 2, 2023

I agree that libc should probably expose the value (so code that checks if it is set in a bitmask works?) but I think it's set to 0x8000 on all linux (64-bit?) architectures so probably can be put in musl/b64/mod.rs (or even in musl/mod.rs, or even even 'linux/mod.rs`?) and have the rest of the definitions removed (see #3261 for other examples of this).

@nekopsykose
Copy link
Contributor Author

but I think it's set to 0x8000 on all linux (64-bit?) architectures

doesn't seem to be (note aarch64, for instance), from musl source:

arch/x32/bits/fcntl.h
16:#define O_LARGEFILE 0100000

arch/powerpc/bits/fcntl.h
16:#define O_LARGEFILE 0200000

arch/s390x/bits/fcntl.h
16:#define O_LARGEFILE 0100000

arch/powerpc64/bits/fcntl.h
16:#define O_LARGEFILE 0200000

arch/mipsn32/bits/fcntl.h
16:#define O_LARGEFILE  020000

arch/m68k/bits/fcntl.h
16:#define O_LARGEFILE 0400000

arch/generic/bits/fcntl.h
16:#define O_LARGEFILE 0100000

arch/aarch64/bits/fcntl.h
16:#define O_LARGEFILE 0400000

arch/mips64/bits/fcntl.h
16:#define O_LARGEFILE  020000

arch/mips/bits/fcntl.h
16:#define O_LARGEFILE  020000

arch/arm/bits/fcntl.h
16:#define O_LARGEFILE 0400000

(see #3261 for other examples of this).

ok, that seems much more thorough as a unification project. perhaps you'd like to also add these constants into that too? although in this case, these don't actually seem consistent for all 32 or all 64 bit widths..

@nekopsykose
Copy link
Contributor Author

(so code that checks if it is set in a bitmask works?)

my initial motivation is that rust 1.70 built on s390x musl now fails without this exposed. i'm not exactly sure why, as the failure is from nix which hasn't changed in forever (this constant is checked here )

perhaps because of some doc feature.. but it indeed is harmless to expose, so hotpatching it for vendor/libc worked fine.

@bossmc
Copy link
Contributor

bossmc commented Jun 2, 2023

Huh, there's more divergence than I expected, don't worry about the unification then!

@nekopsykose
Copy link
Contributor Author

no worries, thanks for the review! always nice to see people working on these interfaces too :)

@JohnTitor
Copy link
Member

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Jun 4, 2023

📌 Commit 4d473b2 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 4, 2023

⌛ Testing commit 4d473b2 with merge 3b808cf...

@bors
Copy link
Contributor

bors commented Jun 4, 2023

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing 3b808cf to main...

@bors bors merged commit 3b808cf into rust-lang:main Jun 4, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants