[PATCH v2] rust: bindings: rename const binding using sed

[PATCH v2] rust: bindings: rename const binding using sed

[Gary Guo]

Current for consts that bindgen don't recognise, we define a helper
constant with

    const <TYPE> BINDINGS_<NAME> = <NAME>;

in `bindings_helper.h` and then we put

    pub const <NAME>: <TYPE> = BINDINGS_<NAME>;

in `bindings/lib.rs`. This is fine that we currently only have 3
constants that are defined this way, but is going to be more annoying
when more constants are added since every new constant needs to be
defined in two places.

This patch changes the way we define constant helpers to

    const <TYPE> RUST_CONST_HELPER_<NAME> = <NAME>;

and then use `sed` to postprocess Rust code by generated by bindgen to
remove the distinct prefix, so user of the binding crate can refer to
the name directly.

Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Signed-off-by: Gary Guo <gary@garyguo.net>
---
v1 -> v2:
- Changed `RUST_BINDING_` to `RUST_CONST_HELPER_`.

 rust/Makefile                   | 2 ++
 rust/bindings/bindings_helper.h | 6 +++---
 rust/bindings/lib.rs            | 3 ---
 rust/kernel/allocator.rs        | 2 +-
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/rust/Makefile b/rust/Makefile
index a27f35f924ec..57f7f5e5a95d 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -337,6 +337,8 @@ quiet_cmd_bindgen = BINDGEN $@
 
 $(obj)/bindings/bindings_generated.rs: private bindgen_target_flags = \
     $(shell grep -Ev '^#|^$$' $(srctree)/$(src)/bindgen_parameters)
+$(obj)/bindings/bindings_generated.rs: private bindgen_target_extra = ; \
+    sed -Ei 's/pub const RUST_CONST_HELPER_([a-zA-Z0-9_]*)/pub const \1/g' $@
 $(obj)/bindings/bindings_generated.rs: $(src)/bindings/bindings_helper.h \
     $(src)/bindgen_parameters FORCE
 	$(call if_changed_dep,bindgen)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index c91a3c24f607..bfa0fa794dae 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -14,6 +14,6 @@
 #include <linux/sched.h>
 
 /* `bindgen` gets confused at certain things. */
-const size_t BINDINGS_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
-const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
-const gfp_t BINDINGS___GFP_ZERO = __GFP_ZERO;
+const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
+const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
+const gfp_t RUST_CONST_HELPER___GFP_ZERO = __GFP_ZERO;
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 9bcbea04dac3..40ddaee50d8b 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -48,6 +48,3 @@ mod bindings_helper {
 }
 
 pub use bindings_raw::*;
-
-pub const GFP_KERNEL: gfp_t = BINDINGS_GFP_KERNEL;
-pub const __GFP_ZERO: gfp_t = BINDINGS___GFP_ZERO;
diff --git a/rust/kernel/allocator.rs b/rust/kernel/allocator.rs
index a8f3d5be1af1..4b057e837358 100644
--- a/rust/kernel/allocator.rs
+++ b/rust/kernel/allocator.rs
@@ -21,7 +21,7 @@ unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: bindings::gf
 
     let mut size = layout.size();
 
-    if layout.align() > bindings::BINDINGS_ARCH_SLAB_MINALIGN {
+    if layout.align() > bindings::ARCH_SLAB_MINALIGN {
         // The alignment requirement exceeds the slab guarantee, thus try to enlarge the size
         // to use the "power-of-two" size/alignment guarantee (see comments in `kmalloc()` for
         // more information).

base-commit: 3857af38e57a80b15b994e19d1f4301cac796481
-- 
2.40.1

Re: [PATCH v2] rust: bindings: rename const binding using sed

[Alice Ryhl]

On 11/4/23 15:56, Gary Guo wrote:
> Current for consts that bindgen don't recognise, we define a helper
> constant with
> 
>      const <TYPE> BINDINGS_<NAME> = <NAME>;
> 
> in `bindings_helper.h` and then we put
> 
>      pub const <NAME>: <TYPE> = BINDINGS_<NAME>;
> 
> in `bindings/lib.rs`. This is fine that we currently only have 3
> constants that are defined this way, but is going to be more annoying
> when more constants are added since every new constant needs to be
> defined in two places.
> 
> This patch changes the way we define constant helpers to
> 
>      const <TYPE> RUST_CONST_HELPER_<NAME> = <NAME>;
> 
> and then use `sed` to postprocess Rust code by generated by bindgen to
> remove the distinct prefix, so user of the binding crate can refer to
> the name directly.
> 
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v2] rust: bindings: rename const binding using sed

[Benno Lossin]

On 04.11.23 15:56, Gary Guo wrote:
> diff --git a/rust/Makefile b/rust/Makefile
> index a27f35f924ec..57f7f5e5a95d 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -337,6 +337,8 @@ quiet_cmd_bindgen = BINDGEN $@
> 
>   $(obj)/bindings/bindings_generated.rs: private bindgen_target_flags = \
>       $(shell grep -Ev '^#|^$$' $(srctree)/$(src)/bindgen_parameters)
> +$(obj)/bindings/bindings_generated.rs: private bindgen_target_extra = ; \
> +    sed -Ei 's/pub const RUST_CONST_HELPER_([a-zA-Z0-9_]*)/pub const \1/g' $@

I think I mentioned this before, but I think this should use `^`
to ensure that this is only replaced at the beginning of lines.

-- 
Cheers,
Benno

Re: [PATCH v2] rust: bindings: rename const binding using sed

[Miguel Ojeda]

On Sat, Nov 4, 2023 at 3:58 PM Gary Guo <gary@garyguo.net> wrote:
>
> This patch changes the way we define constant helpers to
>
>     const <TYPE> RUST_CONST_HELPER_<NAME> = <NAME>;
>
> and then use `sed` to postprocess Rust code by generated by bindgen to
> remove the distinct prefix, so user of the binding crate can refer to
> the name directly.
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Applied to `rust-next` (reworded for typos and with Benno's `^`
suggestion -- we can always relax it back later if needed, the output
is currently the same).

Thanks everyone!

Cheers,
Miguel

Re: [PATCH v2] rust: bindings: rename const binding using sed

[Miguel Ojeda]

On Wed, Dec 13, 2023 at 7:44 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Applied to `rust-next` (reworded for typos and with Benno's `^`
> suggestion -- we can always relax it back later if needed, the output
> is currently the same).

I have dropped the `^` suggestion from `rust-next` since that breaks
non-`rustfmt` builds [1] given everything goes in a single line in the
generated code.

We may want to consider requiring `rustfmt` at some point to simplify things.

[1] https://storage.kernelci.org/next/master/next-20231214/x86_64/x86_64_defconfig+rust/rustc-1.73/logs/kernel.log

Cheers,
Miguel