* 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: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
* 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
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;
as well as URLs for NNTP newsgroup(s).