* Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia)
@ 2004-10-21 9:09 Russell King
2004-10-21 9:31 ` Andrew Morton
2004-10-21 9:40 ` Rusty Russell
0 siblings, 2 replies; 6+ messages in thread
From: Russell King @ 2004-10-21 9:09 UTC (permalink / raw)
To: Linux Kernel List, Rusty Russell, Andrew Morton
It would appear that this change:
-module_param_array(irq_list, int, irq_list_count, 0444);
+module_param_array(irq_list, int, &irq_list_count, 0444);
given:
static int irq_list[16];
static int irq_list_count;
breaks PCMCIA drivers. Why?
#define module_param_array(name, type, num, perm) \
module_param_array_named(name, name, type, num, perm)
#define module_param_array_named(name, array, type, num, perm) \
static struct kparam_array __param_arr_##name \
= { ARRAY_SIZE(array), &num, param_set_##type, param_get_##type,\
sizeof(array[0]), array }; \
module_param_call(name, param_array_set, param_array_get, \
&__param_arr_##name, perm)
Take special note of the '&' before 'num' in the above initialiser, and
check the structure:
struct kparam_array
{
unsigned int max;
unsigned int *num;
param_set_fn set;
param_get_fn get;
unsigned int elemsize;
void *elem;
};
Therefore, module_param_array() does _NOT_ take a pointer to an integer.
Rusty - please fix.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia) 2004-10-21 9:09 Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia) Russell King @ 2004-10-21 9:31 ` Andrew Morton 2004-10-21 9:50 ` Russell King 2004-10-21 9:40 ` Rusty Russell 1 sibling, 1 reply; 6+ messages in thread From: Andrew Morton @ 2004-10-21 9:31 UTC (permalink / raw) To: Russell King; +Cc: linux-kernel, rusty Russell King <rmk+lkml@arm.linux.org.uk> wrote: > > It would appear that this change: > > -module_param_array(irq_list, int, irq_list_count, 0444); > +module_param_array(irq_list, int, &irq_list_count, 0444); > > given: > > static int irq_list[16]; > static int irq_list_count; > > breaks PCMCIA drivers. Why? > > #define module_param_array(name, type, num, perm) \ > module_param_array_named(name, name, type, num, perm) > > #define module_param_array_named(name, array, type, num, perm) \ > static struct kparam_array __param_arr_##name \ > = { ARRAY_SIZE(array), &num, param_set_##type, param_get_##type,\ > sizeof(array[0]), array }; \ > module_param_call(name, param_array_set, param_array_get, \ > &__param_arr_##name, perm) > > Take special note of the '&' before 'num' in the above initialiser, and > check the structure: Something's out of whack with your tree. You should have: #define module_param_array_named(name, array, type, nump, perm) \ static struct kparam_array __param_arr_##name \ = { ARRAY_SIZE(array), nump, param_set_##type, param_get_##type,\ sizeof(array[0]), array }; \ module_param_call(name, param_array_set, param_array_get, \ &__param_arr_##name, perm) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia) 2004-10-21 9:31 ` Andrew Morton @ 2004-10-21 9:50 ` Russell King 2004-10-21 17:02 ` Andrew Morton 2004-10-21 23:46 ` Rusty Russell 0 siblings, 2 replies; 6+ messages in thread From: Russell King @ 2004-10-21 9:50 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, rusty On Thu, Oct 21, 2004 at 02:31:35AM -0700, Andrew Morton wrote: > Russell King <rmk+lkml@arm.linux.org.uk> wrote: > > Take special note of the '&' before 'num' in the above initialiser, and > > check the structure: > > Something's out of whack with your tree. You should have: Ok, but what's the point of the change? If it's to indicate that we're returning a value, shouldn't the other module_param* macros also be fixed in the same way, or do we just like special cases? -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/ 2.6 Serial core ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia) 2004-10-21 9:50 ` Russell King @ 2004-10-21 17:02 ` Andrew Morton 2004-10-21 23:46 ` Rusty Russell 1 sibling, 0 replies; 6+ messages in thread From: Andrew Morton @ 2004-10-21 17:02 UTC (permalink / raw) To: Russell King; +Cc: linux-kernel, rusty Russell King <rmk+lkml@arm.linux.org.uk> wrote: > > On Thu, Oct 21, 2004 at 02:31:35AM -0700, Andrew Morton wrote: > > Russell King <rmk+lkml@arm.linux.org.uk> wrote: > > > Take special note of the '&' before 'num' in the above initialiser, and > > > check the structure: > > > > Something's out of whack with your tree. You should have: > > Ok, but what's the point of the change? If it's to indicate that > we're returning a value, shouldn't the other module_param* macros > also be fixed in the same way, or do we just like special cases? <rusty> module_param_array() takes a variable to put the number of elements in. Looking through the uses, many people don't care, so they declare a dummy or share one variable between several parameters. The latter is problematic because sysfs uses that number to decide how many to display. The solution is to change the variable arg to a pointer, and if the pointer is NULL, use the "max" value. This change is fairly small, but fixing up the callers is a lot of (trivial) churn. </rusty> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia) 2004-10-21 9:50 ` Russell King 2004-10-21 17:02 ` Andrew Morton @ 2004-10-21 23:46 ` Rusty Russell 1 sibling, 0 replies; 6+ messages in thread From: Rusty Russell @ 2004-10-21 23:46 UTC (permalink / raw) To: Russell King; +Cc: Andrew Morton, lkml - Kernel Mailing List On Thu, 2004-10-21 at 19:50, Russell King wrote: > On Thu, Oct 21, 2004 at 02:31:35AM -0700, Andrew Morton wrote: > > Russell King <rmk+lkml@arm.linux.org.uk> wrote: > > > Take special note of the '&' before 'num' in the above initialiser, and > > > check the structure: > > > > Something's out of whack with your tree. You should have: > > Ok, but what's the point of the change? If it's to indicate that > we're returning a value, shouldn't the other module_param* macros > also be fixed in the same way, or do we just like special cases? Only module_param_array() sets a number: the number of elements in the array. By making that arg a pointer, we can put "NULL" there, since it turned out many people didn't care how many elements there were (and were overloading the same variable for all their arrays, which breaks printing in sysfs). Hope that helps, Rusty. -- Anyone who quotes me in their signature is an idiot -- Rusty Russell ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia) 2004-10-21 9:09 Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia) Russell King 2004-10-21 9:31 ` Andrew Morton @ 2004-10-21 9:40 ` Rusty Russell 1 sibling, 0 replies; 6+ messages in thread From: Rusty Russell @ 2004-10-21 9:40 UTC (permalink / raw) To: Russell King; +Cc: Linux Kernel List, Andrew Morton On Thu, 2004-10-21 at 19:09, Russell King wrote: > It would appear that this change: > > -module_param_array(irq_list, int, irq_list_count, 0444); > +module_param_array(irq_list, int, &irq_list_count, 0444); > > given: > > static int irq_list[16]; > static int irq_list_count; > > breaks PCMCIA drivers. Why? > > #define module_param_array(name, type, num, perm) \ > module_param_array_named(name, name, type, num, perm) > > #define module_param_array_named(name, array, type, num, perm) \ > static struct kparam_array __param_arr_##name \ > = { ARRAY_SIZE(array), &num, param_set_##type, param_get_##type,\ > sizeof(array[0]), array }; \ > module_param_call(name, param_array_set, param_array_get, \ > &__param_arr_##name, perm) I'm confused. Andrew, what happened to this part of my patch? diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .22800-linux-2.6-bk/include/linux/moduleparam.h .22800-linux-2.6-bk.updated/include/linux/moduleparam.h --- .22800-linux-2.6-bk/include/linux/moduleparam.h 2004-10-19 14:34:21.000000000 +1000 +++ .22800-linux-2.6-bk.updated/include/linux/moduleparam.h 2004-10-20 17:13:45.000000000 +1000 @@ -129,16 +129,16 @@ extern int param_set_invbool(const char extern int param_get_invbool(char *buffer, struct kernel_param *kp); #define param_check_invbool(name, p) __param_check(name, p, int) -/* Comma-separated array: num is set to number they actually specified. */ -#define module_param_array_named(name, array, type, num, perm) \ +/* Comma-separated array: *nump is set to number they actually specified. */ +#define module_param_array_named(name, array, type, nump, perm) \ static struct kparam_array __param_arr_##name \ - = { ARRAY_SIZE(array), &num, param_set_##type, param_get_##type,\ + = { ARRAY_SIZE(array), nump, param_set_##type, param_get_##type,\ sizeof(array[0]), array }; \ module_param_call(name, param_array_set, param_array_get, \ &__param_arr_##name, perm) -#define module_param_array(name, type, num, perm) \ - module_param_array_named(name, name, type, num, perm) +#define module_param_array(name, type, nump, perm) \ + module_param_array_named(name, name, type, nump, perm) extern int param_array_set(const char *val, struct kernel_param *kp); extern int param_array_get(char *buffer, struct kernel_param *kp); Rusty. -- Anyone who quotes me in their signature is an idiot -- Rusty Russell ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-10-21 23:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-10-21 9:09 Am I paranoid or is everyone out to break my kernel builds (Breakage in drivers/pcmcia) Russell King 2004-10-21 9:31 ` Andrew Morton 2004-10-21 9:50 ` Russell King 2004-10-21 17:02 ` Andrew Morton 2004-10-21 23:46 ` Rusty Russell 2004-10-21 9:40 ` Rusty Russell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox