[PATCH] module parameter array fixes

From: Rusty Russell <rusty@rustcorp.com.au>

Found this while converting netfilter modules to use the new
parameters.  Also fixes an out-by-one error in maximum elements you
can put in array.

The current "intarray" module params were never tested, and um, suck.
Only one person uses them, and it looks painful.

Since noone uses them, replace them with tested versions.

diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
index 055872a858895daaf6f3eb1568b13d97ebbf4a70..89c64b01e8125da8a7ee23c9121a9967b9e65496 100644 (file)
--- a/drivers/eisa/eisa-bus.c
+++ b/drivers/eisa/eisa-bus.c
@@ -33,23 +33,22 @@ static struct eisa_device_info __initdata eisa_table[] = {
 #endif
 
 #define EISA_MAX_FORCED_DEV 16
-#define EISA_FORCED_OFFSET  2
 
-static int enable_dev[EISA_MAX_FORCED_DEV + EISA_FORCED_OFFSET]  = { 1, EISA_MAX_FORCED_DEV, };
-static int disable_dev[EISA_MAX_FORCED_DEV + EISA_FORCED_OFFSET] = { 1, EISA_MAX_FORCED_DEV, };
+static int enable_dev[EISA_MAX_FORCED_DEV];
+static int enable_dev_count;
+static int disable_dev[EISA_MAX_FORCED_DEV];
+static int disable_dev_count;
 
 static int is_forced_dev (int *forced_tab,
+                         int forced_count,
                          struct eisa_root_device *root,
                          struct eisa_device *edev)
 {
        int i, x;
 
-       for (i = 0; i < EISA_MAX_FORCED_DEV; i++) {
-               if (!forced_tab[EISA_FORCED_OFFSET + i])
-                       return 0;
-
+       for (i = 0; i < forced_count; i++) {
                x = (root->bus_nr << 8) | edev->slot;
-               if (forced_tab[EISA_FORCED_OFFSET + i] == x)
+               if (forced_tab[i] == x)
                        return 1;
        }
 
@@ -198,10 +197,10 @@ static int __init eisa_init_device (struct eisa_root_device *root,
 #endif
        }
 
-       if (is_forced_dev (enable_dev, root, edev))
+       if (is_forced_dev (enable_dev, enable_dev_count, root, edev))
                edev->state = EISA_CONFIG_ENABLED | EISA_CONFIG_FORCED;
        
-       if (is_forced_dev (disable_dev, root, edev))
+       if (is_forced_dev (disable_dev, disable_dev_count, root, edev))
                edev->state = EISA_CONFIG_FORCED;
 
        return 0;
@@ -418,12 +417,8 @@ static int __init eisa_init (void)
        return 0;
 }
 
-/* Couldn't use intarray with checking on... :-( */
-#undef  param_check_intarray
-#define param_check_intarray(name, p)
-
-module_param(enable_dev,  intarray, 0444);
-module_param(disable_dev, intarray, 0444);
+module_param_array(enable_dev, int, enable_dev_count, 0444);
+module_param_array(disable_dev, int, disable_dev_count, 0444);
 
 postcore_initcall (eisa_init);
 

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 62127f6f4ad9285c858b3107fe1ef278ecb71476..0a5becbed7748ff29b5cbec73d5ad5899c873d4a 100644 (file)
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -3,6 +3,7 @@
 /* (C) Copyright 2001, 2002 Rusty Russell IBM Corporation */
 #include <linux/init.h>
 #include <linux/stringify.h>
+#include <linux/kernel.h>
 
 /* You can override this manually, but generally this should match the
    module name. */
@@ -33,6 +34,17 @@ struct kparam_string {
        char *string;
 };
 
+/* Special one for arrays */
+struct kparam_array
+{
+       unsigned int max;
+       unsigned int *num;
+       param_set_fn set;
+       param_get_fn get;
+       unsigned int elemsize;
+       void *elem;
+};
+
 /* This is the fundamental function for registering boot/module
    parameters.  perm sets the visibility in driverfs: 000 means it's
    not there, read bits mean it's readable, write bits mean it's
@@ -112,10 +124,16 @@ extern int param_set_invbool(const char *val, struct kernel_param *kp);
 extern int param_get_invbool(char *buffer, struct kernel_param *kp);
 #define param_check_invbool(name, p) __param_check(name, p, int)
 
-/* First two elements are the max and min array length (which don't change) */
-extern int param_set_intarray(const char *val, struct kernel_param *kp);
-extern int param_get_intarray(char *buffer, struct kernel_param *kp);
-#define param_check_intarray(name, p) __param_check(name, p, int *)
+/* Comma-separated array: num is set to number they actually specified. */
+#define module_param_array(name, type, num, perm)                      \
+       static struct kparam_array __param_arr_##name                   \
+       = { ARRAY_SIZE(name), &num, param_set_##type, param_get_##type, \
+           sizeof(name[0]), name };                                    \
+       module_param_call(name, param_array_set, param_array_get,       \
+                         &__param_arr_##name, perm)
+
+extern int param_array_set(const char *val, struct kernel_param *kp);
+extern int param_array_get(char *buffer, struct kernel_param *kp);
 
 extern int param_set_copystring(const char *val, struct kernel_param *kp);
 
@@ -123,5 +141,6 @@ int param_array(const char *name,
                const char *val,
                unsigned int min, unsigned int max,
                void *elem, int elemsize,
-               int (*set)(const char *, struct kernel_param *kp));
+               int (*set)(const char *, struct kernel_param *kp),
+               int *num);
 #endif /* _LINUX_MODULE_PARAM_TYPES_H */

diff --git a/kernel/module.c b/kernel/module.c
index 3741dfa38610249befa102e76f0bb892153f33c7..e7fa84bc231b65d04f7263fe9548af24256d8d1b 100644 (file)
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -844,6 +844,7 @@ int set_obsolete(const char *val, struct kernel_param *kp)
 {
        unsigned int min, max;
        unsigned int size, maxsize;
+       int dummy;
        char *endp;
        const char *p;
        struct obsolete_modparm *obsparm = kp->arg;
@@ -866,19 +867,19 @@ int set_obsolete(const char *val, struct kernel_param *kp)
        switch (*endp) {
        case 'b':
                return param_array(kp->name, val, min, max, obsparm->addr,
-                                  1, param_set_byte);
+                                  1, param_set_byte, &dummy);
        case 'h':
                return param_array(kp->name, val, min, max, obsparm->addr,
-                                  sizeof(short), param_set_short);
+                                  sizeof(short), param_set_short, &dummy);
        case 'i':
                return param_array(kp->name, val, min, max, obsparm->addr,
-                                  sizeof(int), param_set_int);
+                                  sizeof(int), param_set_int, &dummy);
        case 'l':
                return param_array(kp->name, val, min, max, obsparm->addr,
-                                  sizeof(long), param_set_long);
+                                  sizeof(long), param_set_long, &dummy);
        case 's':
                return param_array(kp->name, val, min, max, obsparm->addr,
-                                  sizeof(char *), param_set_charp);
+                                  sizeof(char *), param_set_charp, &dummy);
 
        case 'c':
                /* Undocumented: 1-5c50 means 1-5 strings of up to 49 chars,
@@ -895,7 +896,7 @@ int set_obsolete(const char *val, struct kernel_param *kp)
                if (size >= maxsize) 
                        goto oversize;
                return param_array(kp->name, val, min, max, obsparm->addr,
-                                  maxsize, obsparm_copy_string);
+                                  maxsize, obsparm_copy_string, &dummy);
        }
        printk(KERN_ERR "Unknown obsolete parameter type %s\n", obsparm->type);
        return -EINVAL;

diff --git a/kernel/params.c b/kernel/params.c
index d6a79ffef8bceca3f63950df1a9a277bed98dd55..4d9a71b743c59ae37188ac543c22f98f5d00479d 100644 (file)
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -242,10 +242,10 @@ int param_array(const char *name,
                const char *val,
                unsigned int min, unsigned int max,
                void *elem, int elemsize,
-               int (*set)(const char *, struct kernel_param *kp))
+               int (*set)(const char *, struct kernel_param *kp),
+               int *num)
 {
        int ret;
-       unsigned int count = 0;
        struct kernel_param kp;
        char save;
 
@@ -259,11 +259,12 @@ int param_array(const char *name,
                return -EINVAL;
        }
 
+       *num = 0;
        /* We expect a comma-separated list of values. */
        do {
                int len;
 
-               if (count > max) {
+               if (*num == max) {
                        printk(KERN_ERR "%s: can only take %i arguments\n",
                               name, max);
                        return -EINVAL;
@@ -279,10 +280,10 @@ int param_array(const char *name,
                        return ret;
                kp.arg += elemsize;
                val += len+1;
-               count++;
+               (*num)++;
        } while (save == ',');
 
-       if (count < min) {
+       if (*num < min) {
                printk(KERN_ERR "%s: needs at least %i arguments\n",
                       name, min);
                return -EINVAL;
@@ -290,29 +291,32 @@ int param_array(const char *name,
        return 0;
 }
 
-/* First two elements are the max and min array length (which don't change) */
-int param_set_intarray(const char *val, struct kernel_param *kp)
+int param_array_set(const char *val, struct kernel_param *kp)
 {
-       int *array;
+       struct kparam_array *arr = kp->arg;
 
-       /* Grab min and max as first two elements */
-       array = kp->arg;
-       return param_array(kp->name, val, array[0], array[1], &array[2],
-                          sizeof(int), param_set_int);
+       return param_array(kp->name, val, 1, arr->max, arr->elem,
+                          arr->elemsize, arr->set, arr->num);
 }
 
-int param_get_intarray(char *buffer, struct kernel_param *kp)
+int param_array_get(char *buffer, struct kernel_param *kp)
 {
-       int max;
-       int *array;
-       unsigned int i;
-
-       array = kp->arg;
-       max = array[1];
-
-       for (i = 2; i < max + 2; i++)
-               sprintf(buffer, "%s%i", i > 2 ? "," : "", array[i]);
-       return strlen(buffer);
+       int i, off, ret;
+       struct kparam_array *arr = kp->arg;
+       struct kernel_param p;
+
+       p = *kp;
+       for (i = off = 0; i < *arr->num; i++) {
+               if (i)
+                       buffer[off++] = ',';
+               p.arg = arr->elem + arr->elemsize * i;
+               ret = arr->get(buffer + off, &p);
+               if (ret < 0)
+                       return ret;
+               off += ret;
+       }
+       buffer[off] = '\0';
+       return off;
 }
 
 int param_set_copystring(const char *val, struct kernel_param *kp)
@@ -346,6 +350,6 @@ EXPORT_SYMBOL(param_set_bool);
 EXPORT_SYMBOL(param_get_bool);
 EXPORT_SYMBOL(param_set_invbool);
 EXPORT_SYMBOL(param_get_invbool);
-EXPORT_SYMBOL(param_set_intarray);
-EXPORT_SYMBOL(param_get_intarray);
+EXPORT_SYMBOL(param_array_set);
+EXPORT_SYMBOL(param_array_get);
 EXPORT_SYMBOL(param_set_copystring);