Re: [PATCH] zr36067: Debugging cleanups

On Wed, 23 May 2007, Jean Delvare wrote:
> Hi Ronald,
> On Mon, 7 May 2007 17:43:45 +0200, Jean Delvare wrote:
> > On Mon, 7 May 2007 08:10:31 -0400, Ronald S. Bultje wrote:
> > > On May 7, 2007, at 5:05 AM, Jean Delvare wrote:
> > > > * Use a single global variable to handle the debug level. This makes
> > > >   the driver a bit smaller (by about 800 bytes on x86_64), thanks to
> > > >   one less level of indirection on every use.
> > > [..]
> > > > +int debug = 1;
> > >
> > > This is not good, imagine another driver or kernel particle using a
> > > (global) variable called debug. Last time I did this, there were 3 of
> > > them. :-). Please call it something else.
> >
> > I guess it really only matters when the conflicting drivers are all
> > built-in (as opposed to modular) and I would hope nobody does this,
> > but... You're totally right.
> >
> > > The reason other drivers use debug is b/c their debug is static. The
> > > reason I did this zr_debug + debug hack is b/c it allows the debug
> > > name without having a debug variable.
> >
> > I should have known there was a good reason for this strange construct.
> > Thanks for pointing out the obvious.
> >
> > So I would need to rename the variable to something else, but that
> > breaks backwards compatibility and is also not really aesthetical so...
> > I'd rather not do it and leave things as is, sorry for the noise.
>
> I just remembered that we can use module_param_named() to solve that
> particular problem. This gives us the best of both world: nice and
> unchanged module parameter name, and non-conflicting global variable
> name.

Maybe it was my message from two weeks ago where I suggested doing exactly
that which jogged your memory?

http://thread.gmane.org/gmane.comp.video.mjpeg.devel/2382/focus=2383

zr_debug is probably a poor choice of names for a global variable, since
there are also currently zr36060, zr36120, and zr364xx drivers which could
easily use the same name.

>
> The following patch applies on top of the previous one (although I can
> send a merged patch with all the cleanups if you prefer):

Merged would be nicer.  I was going to ask if you indended to finish this
patch, since I've been fixing bugs in this driver to make it work with
mplayer and tvtime, and fixing the debug printing will conflict.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/