From 7ace64dd8e5f3f0b97bb157c72c712fd81194974 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 23 Feb 2021 02:58:17 +0100 Subject: [PATCH] Formatting support Includes checking in the CI. Signed-off-by: Miguel Ojeda --- .github/workflows/ci.yaml | 3 +++ .gitignore | 1 + .rustfmt.toml | 1 + Documentation/rust/coding.rst | 33 ++++++++++++++++++++++++++++-- Documentation/rust/quick-start.rst | 5 ++--- Makefile | 26 ++++++++++++++++++----- drivers/char/rust_example.rs | 4 ++-- rust/compiler_builtins.rs | 1 - rust/kernel/allocator.rs | 16 +++++++++++++-- rust/kernel/chrdev.rs | 8 ++++++-- rust/kernel/lib.rs | 6 ++++-- rust/kernel/user_ptr.rs | 16 ++++++++++----- rust/module.rs | 7 +++++-- 13 files changed, 101 insertions(+), 26 deletions(-) create mode 100644 .rustfmt.toml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 89fbd854c3f55f..6000a8bc8e042c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -180,3 +180,6 @@ jobs: # Docs - run: make ${{ env.MAKE_ARCH }} ${{ env.MAKE_CROSS_COMPILE }} ${{ env.MAKE_TOOLCHAIN }} ${{ env.MAKE_OUTPUT }} ${{ env.MAKE_SYSROOT }} -j3 rustdoc + + # Formatting + - run: make rustfmtcheck diff --git a/.gitignore b/.gitignore index 8ae4040e373827..a20bd6d5cbcc16 100644 --- a/.gitignore +++ b/.gitignore @@ -95,6 +95,7 @@ modules.order !.gitattributes !.gitignore !.mailmap +!.rustfmt.toml # XXX: Only for GitHub CI -- do not commit into mainline !.github diff --git a/.rustfmt.toml b/.rustfmt.toml new file mode 100644 index 00000000000000..32a9786fa1c4a9 --- /dev/null +++ b/.rustfmt.toml @@ -0,0 +1 @@ +edition = "2018" diff --git a/Documentation/rust/coding.rst b/Documentation/rust/coding.rst index 13586b4cfff76c..b11365b777def8 100644 --- a/Documentation/rust/coding.rst +++ b/Documentation/rust/coding.rst @@ -9,8 +9,37 @@ This document describes how to write Rust code in the kernel. Coding style ------------ -For the moment, we are following the idiomatic Rust style. For instance, -we use 4 spaces for indentation rather than tabs. +The code is automatically formatted using the ``rustfmt`` tool. This is very +good news! + +- If you contribute from time to time to the kernel, you do not need to learn + and remember one more style guide. You will also need less patch roundtrips + to land a change. + +- If you are a reviewer or a maintainer, you will not need to spend time on + pointing out style issues anymore. + +.. note:: Conventions on comments and documentation are not checked by + ``rustfmt``. Thus we still need to take care of those: please see + :ref:`Documentation/rust/docs.rst `. + +We use the tool's default settings. This means we are following the idiomatic +Rust style. For instance, we use 4 spaces for indentation rather than tabs. + +Typically, you will want to instruct your editor/IDE to format while you type, +when you save or at commit time. However, if for some reason you want +to reformat the entire kernel Rust sources at some point, you may run:: + + make rustfmt + +To check if everything is formatted (printing a diff otherwise), e.g. if you +have configured a CI for a tree as a maintainer, you may run:: + + make rustfmtcheck + +Like ``clang-format`` for the rest of the kernel, ``rustfmt`` works on +individual files, and does not require a kernel configuration. Sometimes it may +even work with broken code. Abstractions vs. bindings diff --git a/Documentation/rust/quick-start.rst b/Documentation/rust/quick-start.rst index 7a44d2577d5631..21c4ad35a4f387 100644 --- a/Documentation/rust/quick-start.rst +++ b/Documentation/rust/quick-start.rst @@ -67,9 +67,8 @@ the kernel with ``CC=clang`` or ``LLVM=1``. rustfmt ******* -Optionally, if you install the ``rustfmt`` tool, then the generated C bindings -will be automatically formatted. It is also useful to have the tool to format -your own code, too. +The ``rustfmt`` tool is used to automatically format all the Rust kernel code, +including the generated C bindings. If you are using ``rustup``, its ``default`` profile already installs the tool, so you should be good to go. If you are using another profile, you can install diff --git a/Makefile b/Makefile index 199103e26449bc..0830cffc659c44 100644 --- a/Makefile +++ b/Makefile @@ -264,7 +264,7 @@ no-dot-config-targets := $(clean-targets) \ cscope gtags TAGS tags help% %docs check% coccicheck \ $(version_h) headers headers_% archheaders archscripts \ %asm-generic kernelversion %src-pkg dt_binding_check \ - outputmakefile + outputmakefile rustfmt rustfmtcheck no-sync-config-targets := $(no-dot-config-targets) %install kernelrelease single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.s %.symtypes %/ @@ -445,6 +445,7 @@ READELF = $(CROSS_COMPILE)readelf STRIP = $(CROSS_COMPILE)strip endif RUSTC = rustc +RUSTFMT = rustfmt BINDGEN = bindgen PAHOLE = pahole RESOLVE_BTFIDS = $(objtree)/tools/bpf/resolve_btfids/resolve_btfids @@ -1651,6 +1652,13 @@ help: @echo ' kselftest-merge - Merge all the config dependencies of' @echo ' kselftest to existing .config.' @echo '' + @echo 'Rust targets:' + @echo ' rustfmt - Reformat all the Rust code in the kernel.' + @echo ' rustfmtcheck - Checks if all the Rust code in the kernel' + @echo ' is formatted, printing a diff otherwise.' + @echo ' rustdoc - Generate Rust documentation' + @echo ' (requires kernel .config)' + @echo '' @$(if $(dtstree), \ echo 'Devicetree:'; \ echo '* dtbs - Build device tree blobs for enabled boards'; \ @@ -1669,9 +1677,6 @@ help: @echo 'Documentation targets:' @$(MAKE) -f $(srctree)/Documentation/Makefile dochelp @echo '' - @echo ' rustdoc - Generate Rust documentation' - @echo ' (requires kernel .config)' - @echo '' @echo 'Architecture specific targets ($(SRCARCH)):' @$(if $(archhelp),$(archhelp),\ echo ' No architecture specific help defined for $(SRCARCH)') @@ -1726,14 +1731,25 @@ $(DOC_TARGETS): $(Q)$(MAKE) $(build)=Documentation $@ -# Rust documentation target +# Rust targets # --------------------------------------------------------------------------- +# Documentation target +# # Using the singular to avoid running afoul of `no-dot-config-targets`. PHONY += rustdoc rustdoc: prepare0 $(Q)$(MAKE) $(build)=rust $@ +# Formatting targets +PHONY += rustfmt rustfmtcheck + +rustfmt: + find -name '*.rs' | xargs $(RUSTFMT) + +rustfmtcheck: + find -name '*.rs' | xargs $(RUSTFMT) --check + # Misc # --------------------------------------------------------------------------- diff --git a/drivers/char/rust_example.rs b/drivers/char/rust_example.rs index 5aca2ef621849a..d8d7cc6d0f1fff 100644 --- a/drivers/char/rust_example.rs +++ b/drivers/char/rust_example.rs @@ -73,8 +73,8 @@ impl KernelModule for RustExample { let x: [u64; 1028] = core::hint::black_box([5; 1028]); println!("Large array has length: {}", x.len()); - let mut chrdev_reg = chrdev::Registration::new_pinned( - cstr!("rust_chrdev"), 0, &THIS_MODULE)?; + let mut chrdev_reg = + chrdev::Registration::new_pinned(cstr!("rust_chrdev"), 0, &THIS_MODULE)?; chrdev_reg.as_mut().register::()?; chrdev_reg.as_mut().register::()?; diff --git a/rust/compiler_builtins.rs b/rust/compiler_builtins.rs index 87ec5944121863..5e76f940c3295c 100644 --- a/rust/compiler_builtins.rs +++ b/rust/compiler_builtins.rs @@ -20,7 +20,6 @@ #![feature(compiler_builtins)] #![compiler_builtins] - #![no_builtins] #![no_std] diff --git a/rust/kernel/allocator.rs b/rust/kernel/allocator.rs index 383f85e8046e2c..51e7e806a9c719 100644 --- a/rust/kernel/allocator.rs +++ b/rust/kernel/allocator.rs @@ -40,12 +40,24 @@ pub fn __rust_dealloc(ptr: *mut u8, _size: usize, _align: usize) { #[no_mangle] pub fn __rust_realloc(ptr: *mut u8, _old_size: usize, _align: usize, new_size: usize) -> *mut u8 { - unsafe { bindings::krealloc(ptr as *const c_types::c_void, new_size, bindings::GFP_KERNEL) as *mut u8 } + unsafe { + bindings::krealloc( + ptr as *const c_types::c_void, + new_size, + bindings::GFP_KERNEL, + ) as *mut u8 + } } #[no_mangle] pub fn __rust_alloc_zeroed(size: usize, _align: usize) -> *mut u8 { - unsafe { bindings::krealloc(core::ptr::null(), size, bindings::GFP_KERNEL | bindings::__GFP_ZERO) as *mut u8 } + unsafe { + bindings::krealloc( + core::ptr::null(), + size, + bindings::GFP_KERNEL | bindings::__GFP_ZERO, + ) as *mut u8 + } } #[no_mangle] diff --git a/rust/kernel/chrdev.rs b/rust/kernel/chrdev.rs index 06e0b93fba49ff..3adbe9bce61f14 100644 --- a/rust/kernel/chrdev.rs +++ b/rust/kernel/chrdev.rs @@ -47,7 +47,11 @@ impl Registration<{ N }> { minors_start: u16, this_module: &'static crate::ThisModule, ) -> KernelResult>> { - Ok(Pin::from(Box::try_new(Self::new(name, minors_start, this_module))?)) + Ok(Pin::from(Box::try_new(Self::new( + name, + minors_start, + this_module, + ))?)) } /// Register a character device with this range. Call this once per device /// type (up to `N` times). @@ -57,7 +61,7 @@ impl Registration<{ N }> { if this.inner.is_none() { let mut dev: bindings::dev_t = 0; // SAFETY: Calling unsafe function. `this.name` has 'static - // lifetime + // lifetime let res = unsafe { bindings::alloc_chrdev_region( &mut dev, diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 64e1e124710e15..c0d944ab506058 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -60,7 +60,9 @@ impl ThisModule { // SAFETY: `kernel_param_lock` will check if the pointer is null and use the built-in mutex // in that case. #[cfg(CONFIG_SYSFS)] - unsafe { bindings::kernel_param_lock(self.0) } + unsafe { + bindings::kernel_param_lock(self.0) + } KParamGuard { this_module: self } } @@ -69,7 +71,7 @@ impl ThisModule { /// Scoped lock on the kernel parameters of `ThisModule`. Lock will be released /// when this struct is dropped. pub struct KParamGuard<'a> { - this_module: &'a ThisModule + this_module: &'a ThisModule, } #[cfg(CONFIG_SYSFS)] diff --git a/rust/kernel/user_ptr.rs b/rust/kernel/user_ptr.rs index 2dd0baefd71e4d..d9bba2c31fdd53 100644 --- a/rust/kernel/user_ptr.rs +++ b/rust/kernel/user_ptr.rs @@ -11,11 +11,17 @@ extern "C" { fn rust_helper_access_ok(addr: *const c_types::c_void, len: c_types::c_ulong) -> c_types::c_int; - fn rust_helper_copy_from_user(to: *mut c_types::c_void, from: *const c_types::c_void, - n: c_types::c_ulong) -> c_types::c_ulong; - - fn rust_helper_copy_to_user(to: *mut c_types::c_void, from: *const c_types::c_void, - n: c_types::c_ulong) -> c_types::c_ulong; + fn rust_helper_copy_from_user( + to: *mut c_types::c_void, + from: *const c_types::c_void, + n: c_types::c_ulong, + ) -> c_types::c_ulong; + + fn rust_helper_copy_to_user( + to: *mut c_types::c_void, + from: *const c_types::c_void, + n: c_types::c_ulong, + ) -> c_types::c_ulong; } /// A reference to an area in userspace memory, which can be either diff --git a/rust/module.rs b/rust/module.rs index 97f41b7cbde3a5..1b8311cb761fcd 100644 --- a/rust/module.rs +++ b/rust/module.rs @@ -167,7 +167,7 @@ fn permissions_are_readonly(perms: &str) -> bool { }; match u32::from_str_radix(digits, radix) { Ok(perms) => perms & 0o222 == 0, - Err(_) => false + Err(_) => false, } } @@ -306,7 +306,10 @@ pub fn module(ts: TokenStream) -> TokenStream { _ => ¶m_type, }; let param_default = match param_type.as_ref() { - "str" => format!("b\"{}\0\" as *const _ as *mut kernel::c_types::c_char", param_default), + "str" => format!( + "b\"{}\0\" as *const _ as *mut kernel::c_types::c_char", + param_default + ), _ => param_default, }; let read_func = match (param_type.as_ref(), permissions_are_readonly(¶m_permissions)) {