Merge pull request #93 from Rust-for-Linux/rust-fmt

Formatting support

Author: ojeda

.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

.gitignore

@@ -95,6 +95,7 @@ modules.order
 !.gitattributes
 !.gitignore
 !.mailmap
+!.rustfmt.toml
 
 # XXX: Only for GitHub CI -- do not commit into mainline
 !.github

.rustfmt.toml

@@ -0,0 +1 @@
+edition = "2018"

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 <rust_docs>`.
+
+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

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

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
 # ---------------------------------------------------------------------------

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::<RustFile>()?;
         chrdev_reg.as_mut().register::<RustFile>()?;
 

rust/compiler_builtins.rs

@@ -20,7 +20,6 @@
 
 #![feature(compiler_builtins)]
 #![compiler_builtins]
-
 #![no_builtins]
 #![no_std]
 

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]

rust/kernel/chrdev.rs

@@ -47,7 +47,11 @@ impl<const N: usize> Registration<{ N }> {
         minors_start: u16,
         this_module: &'static crate::ThisModule,
     ) -> KernelResult<Pin<Box<Self>>> {
-        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<const N: usize> 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,

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)]

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

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 {
             _ => &param_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(&param_permissions)) {