On Wed, Mar 4, 2026 at 12:13 AM Petr Pavlu <[email protected]> wrote:
>
> On 2/27/26 12:47 AM, Matthew Wood wrote:
> > Add support for `string` as a parameter type in the module! macro.
> >
> > On the runtime side, add:
> > - set_string_param(): an extern "C" callback matching the
> > kernel_param_ops::set signature that stores the raw C string
> > pointer directly into the SetOnce<StringParam> container, avoiding
> > an unnecessary copy-and-parse round-trip.
> > - PARAM_OPS_STRING: a static kernel_param_ops that uses
> > set_string_param as its setter.
> > - ModuleParam impl for StringParam with try_from_param_arg()
> > returning -EINVAL, since string parameters are populated
> > exclusively through the kernel's set callback.
> >
> > On the macro side:
> > - Change the Parameter::ptype field from Ident to syn::Type to
> > support path-qualified types.
>
> Why is it necessary to change the type of Parameter::ptype? My
> understanding is that this token can currently be "i8", "u8", ...,
> "isize", "usize". Additionally, the value "string" should now be
> accepted. When should one use a path-qualified type in this context?
>
Hi Petr,
Thanks for the review! Yes, I believe your point is correct and I will look
at this again. I think I left that after wanting to be able to use the
StringParam
type, but realized that matching on `string` is more ergonomic.
> > +/// 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 {
>
> The safety comment is somewhat inaccurate because set_param() says that
> the input value needs to be valid only for the duration of the call,
> whereas set_string_param() and StringParam require it to be valid for
> the module's lifetime.
>
Yes, thank you. After reading these safety comments back I agree they need
to be fixed. Miguel had a similar point as well. I'll update these in v2.
Thank you!
- Mat
