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 |