On Fri, Mar 10, 2023 at 03:45:25PM -0800, Luis Chamberlain wrote:
> The routine appldata_register_ops() allocates a sysctl table
> with 4 entries. The firsts one, ops->ctl_table[0] is the parent directory
> with an empty entry following it, ops->ctl_table[1]. The next entry is
> for the the ops->name and that is ops->ctl_table[2]. It needs an empty
for the ops->name
> entry following that, and that is ops->ctl_table[3]. And so hence the
> kcalloc(4, sizeof(struct ctl_table), GFP_KERNEL).
>
> We can simplify this considerably since sysctl_register("foo", table)
> can create the parent directory for us if it does not exist. So we
> can just remove the first two entries and move back the ops->name to
> the first entry, and just use kcalloc(2, ...).
>
> Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> ---
> arch/s390/appldata/appldata_base.c | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c
> index c593f2228083..a60c1e093039 100644
> --- a/arch/s390/appldata/appldata_base.c
> +++ b/arch/s390/appldata/appldata_base.c
> @@ -351,7 +351,8 @@ int appldata_register_ops(struct appldata_ops *ops)
> if (ops->size > APPLDATA_MAX_REC_SIZE)
> return -EINVAL;
>
> - ops->ctl_table = kcalloc(4, sizeof(struct ctl_table), GFP_KERNEL);
> + /* The last entry must be an empty one */
> + ops->ctl_table = kcalloc(2, sizeof(struct ctl_table), GFP_KERNEL);
> if (!ops->ctl_table)
> return -ENOMEM;
>
> @@ -359,17 +360,12 @@ int appldata_register_ops(struct appldata_ops *ops)
> list_add(&ops->list, &appldata_ops_list);
> mutex_unlock(&appldata_ops_mutex);
>
> - ops->ctl_table[0].procname = appldata_proc_name;
> - ops->ctl_table[0].maxlen = 0;
> - ops->ctl_table[0].mode = S_IRUGO | S_IXUGO;
> - ops->ctl_table[0].child = &ops->ctl_table[2];
> + ops->ctl_table[0].procname = ops->name;
> + ops->ctl_table[0].mode = S_IRUGO | S_IWUSR;
> + ops->ctl_table[0].proc_handler = appldata_generic_handler;
> + ops->ctl_table[0].data = ops;
>
> - ops->ctl_table[2].procname = ops->name;
> - ops->ctl_table[2].mode = S_IRUGO | S_IWUSR;
> - ops->ctl_table[2].proc_handler = appldata_generic_handler;
> - ops->ctl_table[2].data = ops;
> -
> - ops->sysctl_header = register_sysctl_table(ops->ctl_table);
> + ops->sysctl_header = register_sysctl(appldata_proc_name, ops->ctl_table);
> if (!ops->sysctl_header)
> goto out;
> return 0;
> --
> 2.39.1
I'll take it with the commit message change mentioned above and the following fixup,
which addresses the obvious problem found during testing:
---
diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c
index a60c1e093039..f462d60679d7 100644
--- a/arch/s390/appldata/appldata_base.c
+++ b/arch/s390/appldata/appldata_base.c
@@ -281,7 +281,7 @@ appldata_generic_handler(struct ctl_table *ctl, int write,
mutex_lock(&appldata_ops_mutex);
list_for_each(lh, &appldata_ops_list) {
tmp_ops = list_entry(lh, struct appldata_ops, list);
- if (&tmp_ops->ctl_table[2] == ctl) {
+ if (&tmp_ops->ctl_table[0] == ctl) {
found = 1;
}
}
--
