Re: [PATCH v3 2/3] can: kvaser_usb: Utilize all possible tx URBs

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2207 bytes --]

On 03/12/2015 11:52 AM, Ahmed S. Darwish wrote:
>>> @@ -1881,7 +1891,7 @@ static int kvaser_usb_init_one(struct usb_interface *intf,
>>>  	if (err)
>>>  		return err;
>>>  
>>> -	netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
>>> +	netdev = alloc_candev(sizeof(*priv), dev->max_tx_urbs);
>>>  	if (!netdev) {
>>>  		dev_err(&intf->dev, "Cannot alloc candev\n");
>>>  		return -ENOMEM;
>>> @@ -1889,6 +1899,13 @@ static int kvaser_usb_init_one(struct usb_interface *intf,
>>>  
>>>  	priv = netdev_priv(netdev);
>>>  
>>> +	priv->tx_contexts = kzalloc(dev->max_tx_urbs *
>>> +				    sizeof(*priv->tx_contexts), GFP_KERNEL);
>>> +	if (!priv->tx_contexts) {
>>> +		free_candev(netdev);
>>> +		return -ENOMEM;
>>> +	}
>>
>> I'm missing a free for the priv->tx_contexts. I see two options:
>>
> 
> Correct. Should not have missed that.
> 
>> 1) use devm_kzalloc(), or
>> 2) move struct kvaser_usb_tx_urb_context tx_contexts[]; to the end of
>>    struct kvaser_usb_net_priv, see [1] for an example.
>>
>>    Without further testing, I think the correct alloc for that case
>>    would be:
>>        alloc_candev(sizeof(*priv + dev->max_tx_urbs *
>>                sizeof(struct kvaser_usb_tx_urb_context))
>>
> 
> The first option looks better I guess. I'll have to check though
> if the resource handling done by devm_kmalloc() will work even
> if the probe() method fails with -ENODEV and the like...

devm does handle failing probe functions by design, while calling a
manual free() on devm allocated mem is a bug, which will cause a double
free or worse.

YMMV: for option 2 saves a mem allocation and a pointer deref for each
access of the context.

>> [1] http://stackoverflow.com/questions/2060974/dynamic-array-in-struct-c

> Thanks for the link. Didn't know that such a "hack" has gained
> official status by C99 :-)

:D and I think it's used in the kernel.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]