Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 26 Jul 2019 11:45:47 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Nishka Dasgupta <nishkadg.linux@gmail.com>
Cc:     lgirdwood@gmail.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: of: Add of_node_put() before return in
 function
Message-ID: <20190726104547.GA4902@sirena.org.uk>
References: <20190724083231.10276-1-nishkadg.linux@gmail.com>
 <20190724154701.GA4524@sirena.org.uk>
 <af559a36-c926-e2a5-a401-aae0f6867a6e@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="C7zPtVaVf+AK4Oqc"
Content-Disposition: inline
In-Reply-To: <af559a36-c926-e2a5-a401-aae0f6867a6e@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--C7zPtVaVf+AK4Oqc
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Jul 26, 2019 at 01:02:52PM +0530, Nishka Dasgupta wrote:
> On 24/07/19 9:17 PM, Mark Brown wrote:
> > On Wed, Jul 24, 2019 at 02:02:31PM +0530, Nishka Dasgupta wrote:

> > > The local variable search in regulator_of_get_init_node takes the value
> > > returned by either of_get_child_by_name or of_node_get, both of which
> > > get a node. If this node is not put before returning, it could cause a
> > > memory leak. Hence put search before a mid-loop return statement.
> > > Issue found with Coccinelle.

> > > -		if (!strcmp(desc->of_match, name))
> > > +		if (!strcmp(desc->of_match, name)) {
> > > +			of_node_put(search);
> > >   			return of_node_get(child);
> > > +		}

> > Why not just remove the extra of_node_get() and a comment explaining why
> > it's not needed?

> I'm sorry, I don't think I understand. I'm putting search in this patch; the
> program was already getting child. Should I also return child directly
> instead of getting it again, and continue to put search?

Your new code is dropping a reference then immediately reacquiring one
to return it (introducing a race condition along the way).  Why not just
return the already held reference and not call any functions at all?

--C7zPtVaVf+AK4Oqc
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl062dgACgkQJNaLcl1U
h9BqfAf/XkPCA/poexRgHnpFE+r37fkrF/5xoqRAFbobrvS42H1YdMPKPDuShBG2
akn59PB6QIRjUgSVJVgdCRvAVDR6/HvvR55SUD66w6iQY3qHIFGxn426t9huP++T
Rox4MQtM4W/6QKb9jXN8fxstr+bvw7LXgXoc5ZHBQ2boMfWormTWe7TtNIp+uZkc
o2Id8iUdXPRk6fwQcRtTAiL3cJpLKWEGh0zK3EB8gFSFolgacoOooY4XeR02lP2T
zmNYrdrGlNPG9ddjKW+RBerO1v8PObzmWmY3hcsTnbhQMnolKklzsJB3egv9FpLW
pbq3pldG5+VJsEHoOmcAnq9DgAUAGQ==
=k/xs
-----END PGP SIGNATURE-----

--C7zPtVaVf+AK4Oqc--