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

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

[Perez-Gonzalez, Inaky]

> From: Greg KH [mailto:greg@kroah.com]
>
> +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)

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

[Greg KH]

On Tue, May 06, 2003 at 10:06:22PM -0700, Perez-Gonzalez, Inaky wrote:
> 
> 
> > From: Greg KH [mailto:greg@kroah.com]
> >
> > +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

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

[Perez-Gonzalez, Inaky]

> From: Greg KH [mailto:greg@kroah.com]
> 
> > 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

Just documenting it should do :]

> 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"  :)

Stab right in the heart :] I think we all agree we prefer "hardened
coders".

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