Skip to content

Commit

Permalink
Merge pull request torvalds#93 from Rust-for-Linux/rust-fmt
Browse files Browse the repository at this point in the history
Formatting support
  • Loading branch information
ojeda authored Feb 23, 2021
2 parents 3012327 + 7ace64d commit 33aa8fa
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 26 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ modules.order
!.gitattributes
!.gitignore
!.mailmap
!.rustfmt.toml

# XXX: Only for GitHub CI -- do not commit into mainline
!.github
Expand Down
1 change: 1 addition & 0 deletions .rustfmt.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
edition = "2018"
33 changes: 31 additions & 2 deletions Documentation/rust/coding.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions Documentation/rust/quick-start.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 21 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 %/

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'; \
Expand All @@ -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)')
Expand Down Expand Up @@ -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
# ---------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions drivers/char/rust_example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>()?;

Expand Down
1 change: 0 additions & 1 deletion rust/compiler_builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

#![feature(compiler_builtins)]
#![compiler_builtins]

#![no_builtins]
#![no_std]

Expand Down
16 changes: 14 additions & 2 deletions rust/kernel/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
8 changes: 6 additions & 2 deletions rust/kernel/chrdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions rust/kernel/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
Expand All @@ -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)]
Expand Down
16 changes: 11 additions & 5 deletions rust/kernel/user_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions rust/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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)) {
Expand Down

0 comments on commit 33aa8fa

Please sign in to comment.