Conversation
|
I assume the failures may be coming from:
|
Account for [1]. [1]: bminor/glibc@3263675
|
The MS_NOUSER fix is easy. Baud rates I'm less sure about; our definitions match those from Linux https://github.com/torvalds/linux/blob/992d4e481e958c6898fe750afd429d1b585fff93/include/uapi/asm-generic/termbits-common.h but glibc doesn't match? https://github.com/bminor/glibc/blob/3fd794264e3f062bfbf0c8727cef82f16d51450b/bits/termios-baud.h. Guessing that the commit above made it so we're starting to see the glibc definitions. |
|
https://sourceware.org/bugzilla/show_bug.cgi?id=33340, totally missed the discussion here at #4692. |
| pub const MS_NOUSER: c_ulong = 0xffffffff80000000; | ||
| // FIXME: should this be an int? The suffix is `U` not `UL`. | ||
| pub const MS_NOUSER: c_ulong = 1 << 31; |
There was a problem hiding this comment.
I wrote the glibc patch. Sorry, I didn't expect breakage here.
Here is my rational from the commit message [1]:
Using gcc -Wshift-overflow=2 -Wsystem-headers to compile a file
including <sys/mount.h> will cause a warning since 1 << 31 is undefined
behavior on platforms where int is 32-bits.
Technically unsigned integers aren't allowed in C enum types, it is a GCC extension that is widely supported. We decided it was safe to use since the extension is already required for EPOLLONESHOT and EPOLLET [2].
Regarding the FIXME, I am not super familiar with Rust, but the mount function uses unsigned long for the flags argument. Until C23 enums could not have a type other than int, otherwise UL probably would have made more sense. Hopefully that helps you a bit in your decision.
[1] https://inbox.sourceware.org/libc-alpha/20250217055647.493638-1-collin.funk1@gmail.com/
[2] https://inbox.sourceware.org/libc-alpha/875xjwvrhj.fsf@oldenburg.str.redhat.com/
There was a problem hiding this comment.
Your patch makes complete sense, I assume the intent was always to be 0x80000000 rather than the sign-extended 0xffffffff80000000. I just left that fixme as something to double check after the experimentation here; surprised you found this PR out of the blue, but thank you for saving me the work :)
I wrote the glibc patch. Sorry, I didn't expect breakage here.
Not at all a problem, there's nothing user-visible—just a test failure checking the equality of constants. Expected whenever the libraries we test against get updated.
|
Related to termio failures: llvm/llvm-project#137403 |
|
Closed to be superseded by a fresher PR #4789. |
4 participants
Cherry picked from #4695