Re: [PATCH 2/8] rust: module_param: wire StringParam into the module! macro

[Matthew Wood]

On Fri, Mar 6, 2026 at 11:28 AM Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote:
>
> On Thu, Feb 26, 2026 at 03:47:28PM -0800, Matthew Wood wrote:
> > +/// Set a string module parameter from a string.
> > +///
> > +/// Similar to [`set_param`] but for [`StringParam`].
> > +///
> > +/// # Safety
> > +///
> > +/// Same requirements as [`set_param`].
> > +unsafe extern "C" fn set_string_param(
> > +    val: *const c_char,
> > +    param: *const bindings::kernel_param,
> > +) -> c_int {
> > +    if val.is_null() {
> > +        crate::pr_warn!("Null pointer passed to `module_param::set_string_param`");
> > +        return EINVAL.to_errno();
> > +    }
> > +
> > +    crate::error::from_result(|| {
> > +        // SAFETY: val points to a valid C string from the kernel.
> > +        let cstr_param = unsafe { StringParam::from_ptr(val) };
> > +
> > +        // SAFETY: By function safety requirements, param.arg points to our SetOnce<StringParam>.
> > +        let container = unsafe { &*((*param).__bindgen_anon_1.arg.cast::<SetOnce<StringParam>>()) };
>
> I do realize this matches set_param, and there's a good chance I
> missed something when reading the macros, but doesn't arg actually
> point to ModuleParamAccess<T> here? Since the struct is not repr(C),
> isn't the compiler technically speaking allowed to reorder the
> fields, which means SetOnce<T> might not actually be at offset 0?
>

Hi Sami,

I appreciate the review! This does seem to be an oversight, I'll try
to figure out the
correct implementation. I agree, `repr(C)` is likely needed here.

> > +
> > +        container
> > +            .populate(cstr_param)
> > +            .then_some(0)
> > +            .ok_or(kernel::error::code::EEXIST)
>
> Does this mean the behavior for Rust modules differs from C modules if
> the user specifies multiple instances of the same parameter? I believe
> we just use the last value of the parameter instead of failing in C.
>
> Sami

Yes, I'll fix that, the implementations should match.

Regards,
Mat