RE: [PATCH net-next 3/6] net: gianfar: allocate queues with devm

From: Claudiu Manoil
Date: Fri Oct 04 2024 - 02:31:37 EST


> -----Original Message-----
> From: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
> Sent: Wednesday, October 2, 2024 10:25 AM
[...]
>
> On Tue, 1 Oct 2024 14:22:01 -0700
> Rosen Penev <rosenp@xxxxxxxxx> wrote:
>
> > There seems to be a mistake here where free_tx_queue is called on
> > failure. Just let devm deal with it.
>
> Good catch, this looks good to me.
>

I like your enthusiasm, but there's nothing to catch here.
kfree() does nothing to NULL objects, however the 'constructor' allocates an
array of objects so free_tx_queues() has to iterate over all objects, to free those
allocated before failure.

I don't have a strong opinion regarding the usage of devm_*() API to allocate resources
at device probing time, it saves some lines of code. However I see this as bringing limited
benefits for simple cases like device probe()/remove(), especially when converting old drivers
like this one. And there's also the risk of falling into the trap of thinking that devm_*() takes
care of everything.

-Claudiu