On Fri, Dec 13, 2024 at 12:33 PM Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:
>
> +#![feature(sync_unsafe_cell)]
Please mention this in the commit message, the status of the feature
and justify the addition.
> +//! C header: [`include/linux/moduleparam.h`](../../../include/linux/moduleparam.h)
Please use `srctree`.
> +/// Newtype to make `bindings::kernel_param` `Sync`.
Please add intra-doc links where applicable, e.g. `Sync` here.
> +unsafe extern "C" fn set_param<T>(
> + val: *const core::ffi::c_char,
> + param: *const crate::bindings::kernel_param,
> +) -> core::ffi::c_int
> +where
> + T: ModuleParam,
> +{
> + // NOTE: If we start supporting arguments without values, val _is_ allowed
> + // to be null here.
> + assert!(!val.is_null());
Should this return an error instead?
> +/// Write a string representation of the current parameter value to `buf`.
> +///
> +/// # Safety
> +///
> +/// Must not be called.
> +///
> +/// # Note
> +///
> +/// This should not be called as we declare all parameters as read only.
> +#[allow(clippy::extra_unused_type_parameters)]
> +unsafe extern "C" fn get_param<T>(
> + _buf: *mut core::ffi::c_char,
> + _param: *const crate::bindings::kernel_param,
> +) -> core::ffi::c_int
> +where
> + T: ModuleParam,
> +{
> + unreachable!("Parameters are not readable");
> +}
Do we need this? Can't the `ops` callback be `null`?
> +/// The `arg` field of `param` must be an initialized instance of `Self`.
`Self`?
> +/// Generate a static [`kernel_param_ops`](../../../include/linux/moduleparam.h) struct.
`srctree`.
> +/// Parse a token stream of the form `expected_name: "value",` and return the
> +/// string in the position of "value". Panics on parse error.
`# Panics` section.
> +/// `type` may be one of
> +///
> +/// - `i8`
> +/// - `u8`
> +/// - `i8`
> +/// - `u8`
> +/// - `i16`
> +/// - `u16`
> +/// - `i32`
> +/// - `u32`
> +/// - `i64`
> +/// - `u64`
> +/// - `isize`
> +/// - `usize`
Can these be intra-doc links?
Thanks!
Cheers,
Miguel
