Messages in this thread |  | | From | Keith Owens <> | Subject | Re: name space analysis and patches, 2.1.17 | Date | Tue, 24 Dec 1996 10:30:27 +1100 |
| |
> > In linux.dev.kernel you write: > > >diff -ur linux-2.1.17.orig/drivers/char/wdt.c linux/drivers/char/wdt.c > >--- linux-2.1.17.orig/drivers/char/wdt.c Fri Dec 13 01:51:09 1996 > >+++ linux/drivers/char/wdt.c Mon Dec 23 14:13:53 1996 > >@@ -48,8 +48,8 @@ > > * You must set these - there is no sane way to probe for this board. > > */ > > > >-int io=0x240; > >-int irq=14; > >+static int io=0x240; > >+static int irq=14; > > > > #define WD_TIMO (100*60) /* 1 minute */ > > Seems you did'nt got the point why they are *not* static: They allow to > give ioport and interrupt while loading a module using something like > > insmod io=0x260 irq=11 wrt.o
Sorry, the syntax is "insmod wdt.o io=0x260 irq=11" and it does work if they are static. I tested this before mailing with "insmod ne.o io=... irq=...", various combinations of io and irq, all were picked up by ne.o and, surprise, surprise, ne.c already contains
static int io[MAX_NE_CARDS] = { 0, }; static int irq[MAX_NE_CARDS] = { 0, };
The majority of modules correctly define their insmod target variables as static, it is only a few that are breaking the convention.
> And probably this is inside a #ifdef MODULE, so you hav'nt problems with > name clashes when compiling it into the kernel...
That is correct to some extent, the linker sees no conflict because the code is compiled as a module. However programming style says that variables should be correctly scoped for humans as well as programs, to quote from Documentation/CodingStyle :-
"GLOBAL variables (to be used only if you _really_ need them) need to have descriptive names, as do global functions. If you have a function that counts the number of active users, you should call that "count_active_users()" or similar, you should _not_ call it "cntusr()". "
CodingStyle does not mention static but (IMHO) a global non-static variable without a *globally* meaningful name is wrong. If it is meant to be globally used, give it a decent name, if it is not meant to be globally used, declare it static.
|  |