Skip to content

Commit 8cbc95f

Browse files
committed
rust: workaround bindgen issue with forward references to enum types
`bindgen` currently generates the wrong type for an `enum` when there is a forward reference to it. For instance: enum E; enum E { A }; generates: pub const E_A: E = 0; pub type E = i32; instead of the expected: pub const E_A: E = 0; pub type E = ffi::c_uint; The issue was reported to upstream `bindgen` [1]. Now, both GCC and Clang support silently these forward references to `enum` types, unless `-Wpedantic` is passed, and it turns out that some headers in the kernel depend on them. Thus, depending on how the headers are included, which in turn may depend on the kernel configuration or the architecture, we may get a different type on the Rust side for a given C `enum`. That can be quite confusing, to say the least, especially since developers may only notice issues when building for other architectures like in [2]. In particular, they may end up forcing a cast and adding an `#[allow(clippy::unnecessary_cast)]` like it was done in commit 94e05a6 ("rust: hrtimer: allow timer restart from timer handler"), which isn't great. Instead, let's have a section at the top of our `bindings_helper.h` that `#include`s the headers with the affected types -- hopefully there are not many cases and there is a single ordering that covers all cases. This allows us to remove the cast and the `#[allow]`, thus keeping the correct code in the source files. When the issue gets resolved in upstream `bindgen` (and we update our minimum `bindgen` version), we can easily remove this section at the top. Link: rust-lang/rust-bindgen#3179 [1] Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2] Acked-by: Andreas Hindborg <[email protected]> Link: https://lore.kernel.org/r/[email protected] [ Added extra paragraph on the comment to clarify that the workaround may not be possible in some cases. - Miguel ] Signed-off-by: Miguel Ojeda <[email protected]>
1 parent cbeaa41 commit 8cbc95f

File tree

2 files changed

+24
-4
lines changed

2 files changed

+24
-4
lines changed

rust/bindings/bindings_helper.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,28 @@
66
* Sorted alphabetically.
77
*/
88

9+
/*
10+
* First, avoid forward references to `enum` types.
11+
*
12+
* This workarounds a `bindgen` issue with them:
13+
* <https://github.com/rust-lang/rust-bindgen/issues/3179>.
14+
*
15+
* Without this, the generated Rust type may be the wrong one (`i32`) or
16+
* the proper one (typically `c_uint`) depending on how the headers are
17+
* included, which in turn may depend on the particular kernel configuration
18+
* or the architecture.
19+
*
20+
* The alternative would be to use casts and likely an
21+
* `#[allow(clippy::unnecessary_cast)]` in the Rust source files. Instead,
22+
* this approach allows us to keep the correct code in the source files and
23+
* simply remove this section when the issue is fixed upstream and we bump
24+
* the minimum `bindgen` version.
25+
*
26+
* This workaround may not be possible in some cases, depending on how the C
27+
* headers are set up.
28+
*/
29+
#include <linux/hrtimer_types.h>
30+
931
#include <kunit/test.h>
1032
#include <linux/blk-mq.h>
1133
#include <linux/blk_types.h>

rust/kernel/time/hrtimer.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,11 +400,9 @@ pub unsafe trait HasHrTimer<T> {
400400
#[repr(u32)]
401401
pub enum HrTimerRestart {
402402
/// Timer should not be restarted.
403-
#[allow(clippy::unnecessary_cast)]
404-
NoRestart = bindings::hrtimer_restart_HRTIMER_NORESTART as u32,
403+
NoRestart = bindings::hrtimer_restart_HRTIMER_NORESTART,
405404
/// Timer should be restarted.
406-
#[allow(clippy::unnecessary_cast)]
407-
Restart = bindings::hrtimer_restart_HRTIMER_RESTART as u32,
405+
Restart = bindings::hrtimer_restart_HRTIMER_RESTART,
408406
}
409407

410408
impl HrTimerRestart {

0 commit comments

Comments
 (0)