On Mon, 2 Aug 2004 17:08:59 +0200
Oliver Neukum <[EMAIL PROTECTED]> wrote:

> > + if (result) {
> > +
> > +       // unlock this device
> > +       up(&dev->sem);
> > +
> > +       // unlock the disconnect semaphore
> > +       up(&disconnect_sem);
> > +       return result;
> > + }
> > 
> > Agreed?
> 
> No, absolutely not. Such a thing makes changing the locks a nightmare
> and leads to unmaintainable code in the long run. The clean solution
> is "goto error_abort;"
> that's the kernel way of simulating exceptions, if you will.

I agree with Oliver, althogh I would not put it into quite as strong terms.
The most important bug in the driver as posted was the use of per-instance
semaphore instead of the global semaphore in the ->open method. The rest
would be nice, but not show-stoppers, IMHO. My personal pet peeve in
the syntax sugar area is the excess empty lines. They degrade the ability
of patch tools to resist double-patching.

-- Pete


-------------------------------------------------------
This SF.Net email is sponsored by OSTG. Have you noticed the changes on
Linux.com, ITManagersJournal and NewsForge in the past few weeks? Now,
one more big change to announce. We are now OSTG- Open Source Technology
Group. Come see the changes on the new OSTG site. www.ostg.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to