Re: [PATCH 2/9] pci: mvebu: fix memory leaks and refcount leaks

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Russell,

On Sat, 03 Oct 2015 19:13:02 +0100, Russell King wrote:

> +	ret = devm_add_action(dev, mvebu_pcie_port_clk_put, port);

I didn't know about devm_add_action(). Definitely very useful for such
situations.

> @@ -1052,10 +1086,12 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
>  		struct mvebu_pcie_port *port = &pcie->ports[i];
>  
>  		ret = mvebu_pcie_parse_port(pcie, port, child);
> -		if (ret < 0)
> +		if (ret < 0) {
> +			of_node_put(child);
>  			return ret;
> -		else if (ret == 0)
> +		} else if (ret == 0) {
>  			continue;
> +		}

This is not trivial. If I understand correctly,
for_each_available_child_of_node() will automatically release the
reference on the previous node and take the reference on the new one
before entering the loop code. So in the skipping case, we don't need
to release the reference as it will be done by the next iteration of
the loop, but in the error case, since we are unexpectedly breaking the
loop, we need to do it manually.

The sort of tricky thing that should be documented near
for_each_child_of_node(), since I believe a lot of code gets this
wrong. See:

   http://lxr.free-electrons.com/source/arch/arm/mach-shmobile/pm-rmobile.c#L367
   http://lxr.free-electrons.com/source/drivers/phy/phy-miphy365x.c#L564

and many more.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com