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