Steven Rostedt <rost...@goodmis.org> writes:
> Currently the params.c code allows only two "set" functions to have
> no arguments. If a parameter does not have an argument, then it
> looks at the set function and tests if it is either param_set_bool()
> or param_set_bint(). If it is not one of these functions, then it
> fails the loading of the module.
>
> But there may be module parameters that have different set functions
> and still allow no arguments. But unless each of these cases adds
> their function to the if statement, it wont be allowed to have no
> arguments. This method gets rather messing and does not scale.
>
> Instead, introduce a flags field to the kernel_param_ops, where if
> the flag KERNEL_PARAM_FL_NOARG is set, the parameter will not fail
> if it does not contain an argument. It will be expected that the
> corresponding set function can handle a NULL pointer as "val".
Good idea. This hack was introduced because people wrote their own
param parsers which didn't expect NULL, leading to oopsen.
A flag is a better solution.
Applied all three.
Thanks,
Rusty.
>
> Signed-off-by: Steven Rostedt <rost...@goodmis.org>
> ---
> include/linux/moduleparam.h | 13 ++++++++++++-
> kernel/params.c | 6 ++++--
> 2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 27d9da3..c3eb102 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -36,7 +36,18 @@ static const char __UNIQUE_ID(name)[]
> \
>
> struct kernel_param;
>
> +/*
> + * Flags available for kernel_param_ops
> + *
> + * NOARG - the parameter allows for no argument (foo instead of foo=1)
> + */
> +enum {
> + KERNEL_PARAM_FL_NOARG = (1 << 0)
> +};
> +
> struct kernel_param_ops {
> + /* How the ops should behave */
> + unsigned int flags;
> /* Returns 0, or -errno. arg is in kp->arg. */
> int (*set)(const char *val, const struct kernel_param *kp);
> /* Returns length written or -errno. Buffer is 4k (ie. be short!) */
> @@ -187,7 +198,7 @@ struct kparam_array
> /* Obsolete - use module_param_cb() */
> #define module_param_call(name, set, get, arg, perm) \
> static struct kernel_param_ops __param_ops_##name = \
> - { (void *)set, (void *)get }; \
> + { 0, (void *)set, (void *)get }; \
> __module_param_call(MODULE_PARAM_PREFIX, \
> name, &__param_ops_##name, arg, \
> (perm) + sizeof(__check_old_set_param(set))*0, -1)
> diff --git a/kernel/params.c b/kernel/params.c
> index 440e65d..27a0af9 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -103,8 +103,8 @@ static int parse_one(char *param,
> || params[i].level > max_level)
> return 0;
> /* No one handled NULL, so do it here. */
> - if (!val && params[i].ops->set != param_set_bool
> - && params[i].ops->set != param_set_bint)
> + if (!val &&
> + !(params[i].ops->flags & KERNEL_PARAM_FL_NOARG))
> return -EINVAL;
> pr_debug("handling %s with %p\n", param,
> params[i].ops->set);
> @@ -320,6 +320,7 @@ int param_get_bool(char *buffer, const struct
> kernel_param *kp)
> EXPORT_SYMBOL(param_get_bool);
>
> struct kernel_param_ops param_ops_bool = {
> + .flags = KERNEL_PARAM_FL_NOARG,
> .set = param_set_bool,
> .get = param_get_bool,
> };
> @@ -370,6 +371,7 @@ int param_set_bint(const char *val, const struct
> kernel_param *kp)
> EXPORT_SYMBOL(param_set_bint);
>
> struct kernel_param_ops param_ops_bint = {
> + .flags = KERNEL_PARAM_FL_NOARG,
> .set = param_set_bint,
> .get = param_get_int,
> };
> --
> 1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
