Re: [linux-usb-devel] [PATCH 7/9] USB usbfs: destroy submitted urbs only on the disconnected interface

[Duncan Sands <baldrick@free.fr>]

> > The "if" cannot be optimized away for the case in point, because it
> > does something (clears the bit) if it passes the test.  If I used WARN_ON
> > then it would have to be WARN_ON(1) in the else branch of the if.
>
> True. You should use BUG_ON().
>
> If this ever happens the device tree is screwed. There's no use going on.

I'm not sure - isn't it more likely that someone stuffed up in usbfs?  BUG_ON
seems kind of harsh, since it will kill the hub thread.  If it wasn't for that I
wouldn't hesitate to use BUG_ON here.

> > > But there is another point. The embedded people deserve a single switch
> > > to remove assertion checks. The purpose of macros like WARN_ON() is
> > > easy and _central_ choice of debugging output vs. kernel size.
> >
> > This is not an argument against using USB's warn, it is an argument for
> > building warn on top of a centralized macro like WARN_ON or a friend.
>
> It is an argument against USB making its own constructs. There's nothing
> terribly specific about USB that would justify it. If the usual debug
> statements are inadequate, improve them.

I don't see that there is anything wrong with USB using it's own constructs even
if they were just defined to be equal to some centralized macro (as they probably
should be).  In fact there is an advantage: they can be modified for debugging
purposes (to add a backtrace for example) without disturbing the rest of the
kernel.

All the best,

Duncan.