On Wed, 9 Jul 2003, Greg KH wrote:

> On Mon, Jul 07, 2003 at 10:20:32AM -0400, Alan Stern wrote:
> > Greg:
> > 
> > This fixes a minor error in usb-skeleton's disconnect() routine: if the 
> > interface's private data is NULL, the current code exits without releasing 
> > the disconnect_sem semaphore.
> 
> Yeah, but if dev is null, then the semaphore is already released, right?

Wrong.  Here's an extract from the current version of the code:

static void skel_disconnect(struct usb_interface *interface)
{
        struct usb_skel *dev;
        int minor;

        /* prevent races with open() */
        down (&disconnect_sem);

        dev = usb_get_intfdata (interface);
        usb_set_intfdata (interface, NULL);

        if (!dev)
                return;

As you can see, disconnect_sem is acquired but not released if dev is 
NULL.

> So yes, the check is being paranoid, but I don't see a problem with
> being paranoid in this kind of situation, do you?

Well, I sort of do.  If the pointer really can never be NULL, then it
doesn't make much difference whether or not you check; the difference is
code size and speed is completely negligible.  There is a difference in
clarity though.  Seeing that check makes me wonder, "What's that doing
there?  How could the pointer ever be NULL?  Is someone just being
paranoid or can that really happen?"  I don't like that kind of
distraction when trying to read code.

On the other hand, suppose there actually is a circumstance in which it
could be NULL.  That could never happen legitimately; it would have to
result from an error somewhere else, maybe a failure of proper locking.
This other error should be corrected, but nobody is likely ever to do so
if they don't know it exists.  Putting that test in there could hide the
existence of a potentially serious error, whereas leaving it out would
give a clear and unambiguous indication that something was really wrong.

Like I said earlier, this is a question of style and taste.  I wonder what
Linus would recommend?

Alan Stern



-------------------------------------------------------
This SF.Net email sponsored by: Parasoft
Error proof Web apps, automate testing & more.
Download & eval WebKing and get a free book.
www.parasoft.com/bulletproofapps
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to