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 |
signature.asc
Description: OpenPGP digital signature
