Re: name space analysis and patches, 2.1.17

Keith Owens (Keith_Owens@ocs.com.au)
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.