2003-05-07 04:53:54

by Perez-Gonzalez, Inaky

Subject: RE: [Bluetooth] HCI USB driver update. Support for SCO over HCI U SB.

> From: Greg KH [mailto:[email protected]]
>
> +int usb_init_urb(struct urb *urb)
> +{
> + if (!urb)
> + return -EINVAL;
> ...
> ...
> ...
> @@ -38,13 +61,14 @@
> mem_flags);
> if (!urb) {
> err("alloc_urb: kmalloc failed");
> - return NULL;
> + goto exit;
> + }
> + if (usb_init_urb(urb)) {
> + kfree(urb);
> + urb = NULL;
> }

If usb_init_urb() is already testing for !urb, why
test it again? No doubt the compiler will probably
catch it if inlining ... but I think the best is
for usb_init_urb() to assume that urb is not NULL.
Let the caller make that sure.

Sorry if this is a dup ... I am catching up with
my mail ...


I?aky P?rez-Gonz?lez -- Not speaking for Intel -- all opinions are my own
(and my fault)

2003-05-07 05:28:53

by Greg KH

Subject: Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI U SB.

On Tue, May 06, 2003 at 10:06:22PM -0700, Perez-Gonzalez, Inaky wrote:
>
>
> > From: Greg KH [mailto:[email protected]]
> >
> > +int usb_init_urb(struct urb *urb)
> > +{
> > + if (!urb)
> > + return -EINVAL;
> > ...
> > ...
> > ...
> > @@ -38,13 +61,14 @@
> > mem_flags);
> > if (!urb) {
> > err("alloc_urb: kmalloc failed");
> > - return NULL;
> > + goto exit;
> > + }
> > + if (usb_init_urb(urb)) {
> > + kfree(urb);
> > + urb = NULL;
> > }
>
> If usb_init_urb() is already testing for !urb, why
> test it again? No doubt the compiler will probably
> catch it if inlining ... but I think the best is
> for usb_init_urb() to assume that urb is not NULL.
> Let the caller make that sure.

Because people other than usb_alloc_urb() can call usb_init_urb().
Yeah, I can remove the check, then any invalid caller will oops on the
first line of usb_init_urb(). I don't mind, was just trying to program
a bit more defensibly. You know, make it a "hardened driver" :)

thanks,

greg k-h