Skip to content

Commit

Permalink
Merge pull request torvalds#847 from ojeda/cleanups
Browse files Browse the repository at this point in the history
treewide: cleanups and clarifications
  • Loading branch information
ojeda authored Aug 1, 2022
2 parents 5f48fa8 + ecd8e1f commit 61104a0
Show file tree
Hide file tree
Showing 45 changed files with 206 additions and 131 deletions.
5 changes: 3 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1716,8 +1716,9 @@ help:
@echo ' rust-analyzer - Generate rust-project.json rust-analyzer support file'
@echo ' (requires kernel .config)'
@echo ' dir/file.[os] - Build specified target only'
@echo ' dir/file.rsi - Build macro expanded source, similar to C preprocessing'
@echo ' (run with RUSTFMT=n to skip reformatting if needed)'
@echo ' dir/file.rsi - Build macro expanded source, similar to C preprocessing.'
@echo ' Run with RUSTFMT=n to skip reformatting if needed.'
@echo ' The output is not intended to be compilable.'
@echo ' dir/file.ll - Build the LLVM assembly file'
@echo ''
@$(if $(dtstree), \
Expand Down
8 changes: 4 additions & 4 deletions drivers/android/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,10 @@ impl Transaction {

impl DeliverToRead for Transaction {
fn do_work(self: Ref<Self>, thread: &Thread, writer: &mut UserSlicePtrWriter) -> Result<bool> {
/* TODO: Initialise the following fields from tr:
pub sender_pid: pid_t,
pub sender_euid: uid_t,
*/
// TODO: Initialise the following fields from `tr`:
// - `pub sender_pid: pid_t`.
// - `pub sender_euid: uid_t`.

let send_failed_reply = ScopeGuard::new(|| {
if self.node_ref.is_some() && self.flags & TF_ONE_WAY == 0 {
let reply = Either::Right(BR_FAILED_REPLY);
Expand Down
4 changes: 4 additions & 0 deletions include/linux/compiler_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

#ifndef __ASSEMBLY__

/*
* Skipped when running bindgen due to a libclang issue;
* see https://github.com/rust-lang/rust-bindgen/issues/2244.
*/
#if defined(CONFIG_DEBUG_INFO_BTF) && defined(CONFIG_PAHOLE_HAS_BTF_TAG) && \
__has_attribute(btf_type_tag) && !defined(__BINDGEN__)
# define BTF_TYPE_TAG(value) __attribute__((btf_type_tag(#value)))
Expand Down
10 changes: 5 additions & 5 deletions include/linux/spinlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,11 @@ static inline void __spin_lock_init(spinlock_t *lock, const char *name,
__raw_spin_lock_init(spinlock_check(lock), name, key, LD_WAIT_CONFIG);
}

# define spin_lock_init(lock) \
do { \
static struct lock_class_key __key; \
\
__spin_lock_init(lock, #lock, &__key); \
# define spin_lock_init(lock) \
do { \
static struct lock_class_key __key; \
\
__spin_lock_init(lock, #lock, &__key); \
} while (0)

#else
Expand Down
4 changes: 2 additions & 2 deletions rust/alloc/README.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# `alloc`

These source files come from the Rust standard library, hosted in
the https://github.com/rust-lang/rust repository, licensed under
the <https://github.com/rust-lang/rust> repository, licensed under
"Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
see https://github.com/rust-lang/rust/blob/master/COPYRIGHT.
see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.

Please note that these files should be kept as close as possible to
upstream. In general, only additions should be performed (e.g. new
Expand Down
2 changes: 2 additions & 0 deletions rust/alloc/boxed/thin.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

// Based on
// https://github.com/matthieu-m/rfc2580/blob/b58d1d3cba0d4b5e859d3617ea2d0943aaa31329/examples/thin.rs
// by matthieu-m
Expand Down
2 changes: 2 additions & 0 deletions rust/alloc/ffi/c_str.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

#[cfg(test)]
mod tests;

Expand Down
2 changes: 2 additions & 0 deletions rust/alloc/ffi/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

//! Utilities related to FFI bindings.
//!
//! This module provides utilities to handle data across non-Rust
Expand Down
2 changes: 1 addition & 1 deletion rust/bindings/bindings_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <linux/errname.h>
#include <linux/file.h>
#include <linux/fs.h>
#include <linux/fs_parser.h>
#include <linux/gpio/driver.h>
#include <linux/hw_random.h>
#include <linux/interrupt.h>
Expand All @@ -36,7 +37,6 @@
#include <linux/uaccess.h>
#include <linux/uio.h>
#include <uapi/linux/android/binder.h>
#include <linux/fs_parser.h>

/* `bindgen` gets confused at certain things. */
const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
Expand Down
2 changes: 1 addition & 1 deletion rust/bindings/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#![no_std]
#![feature(core_ffi_c)]
// See https://github.com/rust-lang/rust-bindgen/issues/1651.
// See <https://github.com/rust-lang/rust-bindgen/issues/1651>.
#![cfg_attr(test, allow(deref_nullptr))]
#![cfg_attr(test, allow(unaligned_references))]
#![cfg_attr(test, allow(unsafe_op_in_unsafe_fn))]
Expand Down
2 changes: 1 addition & 1 deletion rust/exports.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

#include <linux/module.h>

#define EXPORT_SYMBOL_RUST_GPL(sym) extern int sym; EXPORT_SYMBOL_GPL(sym);
#define EXPORT_SYMBOL_RUST_GPL(sym) extern int sym; EXPORT_SYMBOL_GPL(sym)

#include "exports_core_generated.h"
#include "exports_alloc_generated.h"
Expand Down
2 changes: 1 addition & 1 deletion rust/helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <linux/build_bug.h>
#include <linux/clk.h>
#include <linux/errname.h>
#include <linux/fs_parser.h>
#include <linux/gfp.h>
#include <linux/highmem.h>
#include <linux/io.h>
Expand All @@ -38,7 +39,6 @@
#include <linux/skbuff.h>
#include <linux/uaccess.h>
#include <linux/uio.h>
#include <linux/fs_parser.h>

__noreturn void rust_helper_BUG(void)
{
Expand Down
12 changes: 8 additions & 4 deletions rust/kernel/amba.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@ unsafe extern "C" fn probe_callback<T: Driver>(
} else {
// SAFETY: The offset comes from a previous call to `offset_from` in `IdArray::new`,
// which guarantees that the resulting pointer is within the table.
let ptr = unsafe { aid.cast::<u8>().offset(offset as _).cast::<Option<T::IdInfo>>() };
let ptr = unsafe {
aid.cast::<u8>()
.offset(offset as _)
.cast::<Option<T::IdInfo>>()
};
// SAFETY: The id table has a static lifetime, so `ptr` is guaranteed to be valid for
// read.
unsafe { (&*ptr).as_ref() }
Expand Down Expand Up @@ -243,9 +247,9 @@ macro_rules! module_amba_driver {
/// # fn probe(_dev: &mut amba::Device, _id: Option<&Self::IdInfo>) -> Result {
/// # Ok(())
/// # }
/// define_amba_id_table! {(), [
/// ({ id: 0x00041061, mask: 0x000fffff }, None),
/// ]}
/// define_amba_id_table! {(), [
/// ({ id: 0x00041061, mask: 0x000fffff }, None),
/// ]}
/// # }
/// ```
#[macro_export]
Expand Down
1 change: 1 addition & 0 deletions rust/kernel/build_assert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
/// be called, a build error will be triggered.
///
/// # Examples
///
/// ```
/// # use kernel::build_error;
/// #[inline]
Expand Down
2 changes: 1 addition & 1 deletion rust/kernel/delay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn coarse_sleep_conversion(duration: Duration) -> core::ffi::c_uint {
/// coarse_sleep(Duration::new(1, 2)); // Equivalent to `msleep(1001)`.
/// ```
pub fn coarse_sleep(duration: Duration) {
// SAFETY: msleep is safe for all values of an `unsigned int`.
// SAFETY: `msleep` is safe for all values of its argument.
unsafe { bindings::msleep(coarse_sleep_conversion(duration)) }
}

Expand Down
5 changes: 3 additions & 2 deletions rust/kernel/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ use crate::c_str;
/// related to `self`, that is, actions on it will affect `self`. For example, if one calls
/// `get_device`, then the refcount on the device represented by `self` will be incremented.
///
/// Additionally, implementers must ensure that the device is never renamed. Commit a5462516aa994
/// has details on why `device_rename` should not be used.
/// Additionally, implementers must ensure that the device is never renamed. Commit a5462516aa99
/// ("driver-core: document restrictions on device_rename()") has details on why `device_rename`
/// should not be used.
pub unsafe trait RawDevice {
/// Returns the raw `struct device` related to `self`.
fn raw_device(&self) -> *mut bindings::device;
Expand Down
12 changes: 6 additions & 6 deletions rust/kernel/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,16 +280,16 @@ macro_rules! first_item {
/// assert_eq!([] as [u32; 0], second_item!());
/// assert_eq!([10u32], second_item!((X, 10u32)));
/// assert_eq!([10u32], second_item!((X, 10u32),));
/// assert_eq!([10u32], second_item!(({X}, 10u32)));
/// assert_eq!([10u32], second_item!(({X}, 10u32),));
/// assert_eq!([10u32], second_item!(({ X }, 10u32)));
/// assert_eq!([10u32], second_item!(({ X }, 10u32),));
/// assert_eq!([10u32, 20], second_item!((X, 10u32), (X, 20)));
/// assert_eq!([10u32, 20], second_item!((X, 10u32), (X, 20),));
/// assert_eq!([10u32, 20], second_item!(({X}, 10u32), ({X}, 20)));
/// assert_eq!([10u32, 20], second_item!(({X}, 10u32), ({X}, 20),));
/// assert_eq!([10u32, 20], second_item!(({ X }, 10u32), ({ X }, 20)));
/// assert_eq!([10u32, 20], second_item!(({ X }, 10u32), ({ X }, 20),));
/// assert_eq!([10u32, 20, 30], second_item!((X, 10u32), (X, 20), (X, 30)));
/// assert_eq!([10u32, 20, 30], second_item!((X, 10u32), (X, 20), (X, 30),));
/// assert_eq!([10u32, 20, 30], second_item!(({X}, 10u32), ({X}, 20), ({X}, 30)));
/// assert_eq!([10u32, 20, 30], second_item!(({X}, 10u32), ({X}, 20), ({X}, 30),));
/// assert_eq!([10u32, 20, 30], second_item!(({ X }, 10u32), ({ X }, 20), ({ X }, 30)));
/// assert_eq!([10u32, 20, 30], second_item!(({ X }, 10u32), ({ X }, 20), ({ X }, 30),));
/// ```
#[macro_export]
macro_rules! second_item {
Expand Down
30 changes: 20 additions & 10 deletions rust/kernel/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub mod flags {
/// Ensure that this file is created with the `open(2)` call.
pub const O_EXCL: u32 = bindings::O_EXCL;

/// Large file size enabled (`off64_t` over `off_t`)
/// Large file size enabled (`off64_t` over `off_t`).
pub const O_LARGEFILE: u32 = bindings::O_LARGEFILE;

/// Do not update the file last access time.
Expand Down Expand Up @@ -326,15 +326,16 @@ impl<A: OpenAdapter<T::OpenData>, T: Operations> OperationsVtable<A, T> {
offset: *mut bindings::loff_t,
) -> core::ffi::c_ssize_t {
from_kernel_result! {
let mut data = unsafe { UserSlicePtr::new(buf as *mut core::ffi::c_void, len).writer() };
let mut data =
unsafe { UserSlicePtr::new(buf as *mut core::ffi::c_void, len).writer() };
// SAFETY: `private_data` was initialised by `open_callback` with a value returned by
// `T::Data::into_pointer`. `T::Data::from_pointer` is only called by the
// `release` callback, which the C API guarantees that will be called only when all
// references to `file` have been released, so we know it can't be called while this
// function is running.
let f = unsafe { T::Data::borrow((*file).private_data) };
// No `FMODE_UNSIGNED_OFFSET` support, so `offset` must be in [0, 2^63).
// See discussion in https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113
// See <https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113>.
let read = T::read(
f,
unsafe { File::from_ptr(file) },
Expand All @@ -360,8 +361,12 @@ impl<A: OpenAdapter<T::OpenData>, T: Operations> OperationsVtable<A, T> {
// references to `file` have been released, so we know it can't be called while this
// function is running.
let f = unsafe { T::Data::borrow((*file).private_data) };
let read =
T::read(f, unsafe { File::from_ptr(file) }, &mut iter, offset.try_into()?)?;
let read = T::read(
f,
unsafe { File::from_ptr(file) },
&mut iter,
offset.try_into()?,
)?;
unsafe { (*iocb).ki_pos += bindings::loff_t::try_from(read).unwrap() };
Ok(read as _)
}
Expand All @@ -374,20 +379,21 @@ impl<A: OpenAdapter<T::OpenData>, T: Operations> OperationsVtable<A, T> {
offset: *mut bindings::loff_t,
) -> core::ffi::c_ssize_t {
from_kernel_result! {
let mut data = unsafe { UserSlicePtr::new(buf as *mut core::ffi::c_void, len).reader() };
let mut data =
unsafe { UserSlicePtr::new(buf as *mut core::ffi::c_void, len).reader() };
// SAFETY: `private_data` was initialised by `open_callback` with a value returned by
// `T::Data::into_pointer`. `T::Data::from_pointer` is only called by the
// `release` callback, which the C API guarantees that will be called only when all
// references to `file` have been released, so we know it can't be called while this
// function is running.
let f = unsafe { T::Data::borrow((*file).private_data) };
// No `FMODE_UNSIGNED_OFFSET` support, so `offset` must be in [0, 2^63).
// See discussion in https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113
// See <https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113>.
let written = T::write(
f,
unsafe { File::from_ptr(file) },
&mut data,
unsafe { *offset }.try_into()?
unsafe { *offset }.try_into()?,
)?;
unsafe { (*offset) += bindings::loff_t::try_from(written).unwrap() };
Ok(written as _)
Expand All @@ -408,8 +414,12 @@ impl<A: OpenAdapter<T::OpenData>, T: Operations> OperationsVtable<A, T> {
// references to `file` have been released, so we know it can't be called while this
// function is running.
let f = unsafe { T::Data::borrow((*file).private_data) };
let written =
T::write(f, unsafe { File::from_ptr(file) }, &mut iter, offset.try_into()?)?;
let written = T::write(
f,
unsafe { File::from_ptr(file) },
&mut iter,
offset.try_into()?,
)?;
unsafe { (*iocb).ki_pos += bindings::loff_t::try_from(written).unwrap() };
Ok(written as _)
}
Expand Down
10 changes: 7 additions & 3 deletions rust/kernel/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,10 @@ impl<T: Type + ?Sized> Tables<T> {
// SAFETY: The callback contract guarantees the parameter key to be valid and last at
// least the duration of the callback.
T::Context::parse_unknown_param(
&mut data, unsafe { CStr::from_char_ptr(param.key) }, val)?;
&mut data,
unsafe { CStr::from_char_ptr(param.key) },
val,
)?;
Ok(0)
}
}
Expand Down Expand Up @@ -291,8 +294,9 @@ impl<T: Type + ?Sized> Tables<T> {
// SAFETY: Both `fc` and `buf` are guaranteed to be valid; the former because the
// callback is still ongoing and the latter because its lifefime is tied to that of
// `page`, which is also valid for the duration of the callback.
to_result(
unsafe { bindings::generic_parse_monolithic(fc, buf.as_mut_ptr().cast()) })?;
to_result(unsafe {
bindings::generic_parse_monolithic(fc, buf.as_mut_ptr().cast())
})?;
}
Ok(0)
}
Expand Down
15 changes: 8 additions & 7 deletions rust/kernel/fs/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,12 @@ impl<S, T: Fn(&mut S, &bindings::fs_parameter, &bindings::fs_parse_result) -> Re
/// # use kernel::count_brace_items;
///
/// assert_eq!(0, count_brace_items!());
/// assert_eq!(1, count_brace_items!({A}));
/// assert_eq!(1, count_brace_items!({A},));
/// assert_eq!(2, count_brace_items!({A}, {B}));
/// assert_eq!(2, count_brace_items!({A}, {B},));
/// assert_eq!(3, count_brace_items!({A}, {B}, {C}));
/// assert_eq!(3, count_brace_items!({A}, {B}, {C},));
/// assert_eq!(1, count_brace_items!({ A }));
/// assert_eq!(1, count_brace_items!({ A },));
/// assert_eq!(2, count_brace_items!({ A }, { B }));
/// assert_eq!(2, count_brace_items!({ A }, { B },));
/// assert_eq!(3, count_brace_items!({ A }, { B }, { C }));
/// assert_eq!(3, count_brace_items!({ A }, { B }, { C },));
/// ```
#[macro_export]
macro_rules! count_brace_items {
Expand All @@ -434,6 +434,7 @@ macro_rules! count_brace_items {
/// Defines the file system parameters of a given file system context.
///
/// # Examples
///
/// ```
/// # use kernel::prelude::*;
/// # use kernel::{c_str, fs, str::CString};
Expand Down Expand Up @@ -461,7 +462,7 @@ macro_rules! count_brace_items {
/// impl fs::Context<Self> for Example {
/// type Data = Box<State>;
///
/// kernel::define_fs_params!{Box<State>,
/// kernel::define_fs_params! {Box<State>,
/// {flag, "flag", |s, v| { s.flag = Some(v); Ok(()) } },
/// {flag_no, "flagno", |s, v| { s.flag_no = Some(v); Ok(()) } },
/// {bool, "bool", |s, v| { s.bool_value = Some(v); Ok(()) } },
Expand Down
5 changes: 4 additions & 1 deletion rust/kernel/gpio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ pub trait Chip {
///
/// ```
/// # use kernel::prelude::*;
/// use kernel::{device::RawDevice, gpio::{self, Registration}};
/// use kernel::{
/// device::RawDevice,
/// gpio::{self, Registration},
/// };
///
/// struct MyGpioChip;
/// #[vtable]
Expand Down
3 changes: 2 additions & 1 deletion rust/kernel/hwrng.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ impl<T: Operations> Registration<T> {

let name = CString::try_from_fmt(name)?;

// SAFETY: Registration is pinned and contains allocated and set to zero `bindings::hwrng` structure.
// SAFETY: Registration is pinned and contains allocated and set to zero
// `bindings::hwrng` structure.
Self::init_hwrng(
unsafe { &mut *this.hwrng.get() },
&name,
Expand Down
1 change: 0 additions & 1 deletion rust/kernel/io_mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ impl Resource {
///
/// Ok(())
/// }
///
/// ```
pub struct IoMem<const SIZE: usize> {
ptr: usize,
Expand Down
Loading

0 comments on commit 61104a0

Please sign in to comment.