the usage of DEBUG_DRIVER seems ambiguous

the usage of DEBUG_DRIVER seems ambiguous

[Robert P. J. Day]

  the usage of the DEBUG_DRIVER preprocessor variable is a big
confusing:

$ $ grep -rw DEBUG_DRIVER *
drivers/net/sunlance.c:#undef DEBUG_DRIVER
drivers/net/a2065.c:#ifdef DEBUG_DRIVER
drivers/net/a2065.c:#ifdef DEBUG_DRIVER
drivers/net/7990.c:#ifdef DEBUG_DRIVER
drivers/net/7990.c:#ifdef DEBUG_DRIVER
drivers/base/Kconfig:config DEBUG_DRIVER
...

  it's clearly a configuration variable, but it's also being used by
itself in a few drivers/net/ source files.  is that deliberate?

rday

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

Re: the usage of DEBUG_DRIVER seems ambiguous

[Randy Dunlap]

On Wed, 7 Mar 2007 19:44:41 -0500 (EST) Robert P. J. Day wrote:

> 
>   the usage of the DEBUG_DRIVER preprocessor variable is a big
> confusing:
> 
> $ $ grep -rw DEBUG_DRIVER *
> drivers/net/sunlance.c:#undef DEBUG_DRIVER
> drivers/net/a2065.c:#ifdef DEBUG_DRIVER
> drivers/net/a2065.c:#ifdef DEBUG_DRIVER
> drivers/net/7990.c:#ifdef DEBUG_DRIVER
> drivers/net/7990.c:#ifdef DEBUG_DRIVER
> drivers/base/Kconfig:config DEBUG_DRIVER
> ...
> 
>   it's clearly a configuration variable, but it's also being used by
> itself in a few drivers/net/ source files.  is that deliberate?

The ones in drivers/net/ are just local driver debug controls.
They happen to have the same name as a (likely newer) kconfig symbol.

Is there a real problem that needs to be fixed?

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: the usage of DEBUG_DRIVER seems ambiguous

[Robert P. J. Day]

On Thu, 8 Mar 2007, Randy Dunlap wrote:

> On Wed, 7 Mar 2007 19:44:41 -0500 (EST) Robert P. J. Day wrote:
>
> >
> >   the usage of the DEBUG_DRIVER preprocessor variable is a big
> > confusing:
> >
> > $ $ grep -rw DEBUG_DRIVER *
> > drivers/net/sunlance.c:#undef DEBUG_DRIVER
> > drivers/net/a2065.c:#ifdef DEBUG_DRIVER
> > drivers/net/a2065.c:#ifdef DEBUG_DRIVER
> > drivers/net/7990.c:#ifdef DEBUG_DRIVER
> > drivers/net/7990.c:#ifdef DEBUG_DRIVER
> > drivers/base/Kconfig:config DEBUG_DRIVER
> > ...
> >
> >   it's clearly a configuration variable, but it's also being used by
> > itself in a few drivers/net/ source files.  is that deliberate?
>
> The ones in drivers/net/ are just local driver debug controls.
> They happen to have the same name as a (likely newer) kconfig symbol.
>
> Is there a real problem that needs to be fixed?

nope, if they were deliberately named that way, and weren't meant to
be associated with the Kconfig variable of that same name.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

Re: the usage of DEBUG_DRIVER seems ambiguous

[Artem Bityutskiy]

Randy Dunlap wrote:
>>   it's clearly a configuration variable, but it's also being used by
>> itself in a few drivers/net/ source files.  is that deliberate?
> 
> The ones in drivers/net/ are just local driver debug controls.
> They happen to have the same name as a (likely newer) kconfig symbol.
> 
> Is there a real problem that needs to be fixed?

Renaming them just for the sake of being less confusing makes sense.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: the usage of DEBUG_DRIVER seems ambiguous

[Robert P. J. Day]

On Fri, 9 Mar 2007, Artem Bityutskiy wrote:

> Randy Dunlap wrote:
> > >   it's clearly a configuration variable, but it's also being used by
> > > itself in a few drivers/net/ source files.  is that deliberate?
> >
> > The ones in drivers/net/ are just local driver debug controls.
> > They happen to have the same name as a (likely newer) kconfig symbol.
> >
> > Is there a real problem that needs to be fixed?
>
> Renaming them just for the sake of being less confusing makes sense.

that's kind of what i had in mind.  i have a script that peruses the
source tree, checking for apparent typoes in preprocessor directives
when someone forgets the leading "CONFIG_", and as long as that macro
name is the way it is, that example is going to be flagged every time
for no good reason.

if someone wants to make a suggestion, i can submit a simple renaming
patch.

rday

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

Re: the usage of DEBUG_DRIVER seems ambiguous

[Stefan Richter]

Robert P. J. Day wrote:
> On Fri, 9 Mar 2007, Artem Bityutskiy wrote:
>> Randy Dunlap wrote:
>> > The ones in drivers/net/ are just local driver debug controls.
>> > They happen to have the same name as a (likely newer) kconfig symbol.
>> >
>> > Is there a real problem that needs to be fixed?
>>
>> Renaming them just for the sake of being less confusing makes sense.
...
> if someone wants to make a suggestion, i can submit a simple renaming
> patch.

If a driver or subsystem already uses a prefix to have an own namespace
for macros, functions, structs and so on, a local DEBUG_DRIVER could
become something like LOCALPREFIX_DEBUG. If there is a narrow usage of
the macro, e.g. to indicate a debug level, it could become something
speaking like LOCALPREFIX_DEBUG_LEVEL. --- However, after looking at the
actual occurrences of DEBUG_DRIVER, I see that this recommendation
doesn't really apply that well.

>From your initial post:
| $ $ grep -rw DEBUG_DRIVER *
| drivers/net/sunlance.c:#undef DEBUG_DRIVER

This is an old forgotten rest of earlier debug code. See here for evidence:
http://lxr.linux.no/source/drivers/net/sunlance.c?v=2.2.26#L791
791 #ifdef DEBUG_DRIVER
792         printk (KERN_DEBUG "Lance restart=%d\n", status);
793 #endif

This usage of DEBUG_DRIVER isn't there anymore. Therefore simply delete
the remaining occurrence:
-#undef DEBUG_DRIVER

| drivers/net/a2065.c:#ifdef DEBUG_DRIVER
| drivers/net/a2065.c:#ifdef DEBUG_DRIVER

Rename to A2065_DEBUG or LANCE_DEBUG. Two more alternatives:

-#ifdef DEBUG_DRIVER
+#if 0 /* debug */

or
-#ifdef DEBUG_DRIVER
-	years_old_debug_cruft_nobody_enables_anymore();
-#endif

Needless to say, the maintainer certainly wants to ACK/NAK this.

| drivers/net/7990.c:#ifdef DEBUG_DRIVER
| drivers/net/7990.c:#ifdef DEBUG_DRIVER

Exactly like with a2065.c.

| drivers/base/Kconfig:config DEBUG_DRIVER

According to where it is defined, CONFIG_DEBUG_DRIVER should only occur
in drivers/base/* (and some defconfigs outside of drivers/base/).

| ...

More hits from LXR:

drivers/isdn/hardware/eicon/dbgioctl.h

Where is this header file used anyway? Can the entire file be deleted?

drivers/isdn/gigaset/gigaset.h  (definition as enum item)
drivers/isdn/gigaset/common.c   (multiple uses)

If the overlap with CONFIG_DEBUG_DRIVER bothers you, rename it to
DEBUG_DRIVER_STRUCT or whatever.
-- 
Stefan Richter
-=====-=-=== --== --===
http://arcgraph.de/sr/