Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Jesper Juhl <jesper.juhl@gmail.com>]

On 9/4/05, Horst von Brand <vonbrand@inf.utfsm.cl> wrote:
> Jesper Juhl <jesper.juhl@gmail.com> wrote:
> > On 9/4/05, Harald Welte <laforge@gnumonks.org> wrote:
> > > On Sun, Sep 04, 2005 at 12:12:18PM +0200, Harald Welte wrote:
> > > > Hi!
> > > >
> > > > Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> > > > Smartcard Reader.
> > >
> > > Sorry, the patch was missing a "cg-add" of the header file.  Please use
> > > the patch below.
> >
> > It would be so much nicer if the patch actually was "below" - that is
> > "inline in the email as opposed to as an attachment". Having to first
> > save an attachment and then cut'n'paste from it is a pain.
> >
> > Anyway, a few comments below :
> 
> [...]
> 
> > +     unsigned long ulBytesToRead;
> >
> >
> > lowercase prefered also for variables.
> 
> Also, "encoding" the type (ul) into the variable name is nonsense.
> 
Agreed, and it's even mentioned in CodingStyle (ok, it talks about
functions, but the same goes for variables):

...
Encoding the type of a function into the name (so-called Hungarian
notation) is brain damaged - the compiler knows the types anyway and can
check those, and it only confuses the programmer.  No wonder MicroSoft
makes buggy programs.

LOCAL variable names should be short, and to the point.  If you have
some random integer loop counter, it should probably be called "i".
Calling it "loop_counter" is non-productive, if there is no chance of it
being mis-understood.  Similarly, "tmp" can be just about any type of
variable that is used to hold a temporary value.
...



-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html