From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=tq6k=PW=vger.kernel.org=linux-kernel-owner@kernel.org>
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
	aws-us-west-2-korg-lkml-1.web.codeaurora.org
X-Spam-Level: 
X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS,
	INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED
	autolearn=unavailable autolearn_force=no version=3.4.0
Received: from mail.kernel.org (mail.kernel.org [198.145.29.99])
	by smtp.lore.kernel.org (Postfix) with ESMTP id 11565C43612
	for <linux-kernel@archiver.kernel.org>; Mon, 14 Jan 2019 16:19:55 +0000 (UTC)
Received: from vger.kernel.org (vger.kernel.org [209.132.180.67])
	by mail.kernel.org (Postfix) with ESMTP id E047620873
	for <linux-kernel@archiver.kernel.org>; Mon, 14 Jan 2019 16:19:54 +0000 (UTC)
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1726777AbfANQTy (ORCPT
        <rfc822;linux-kernel@archiver.kernel.org>);
        Mon, 14 Jan 2019 11:19:54 -0500
Received: from mail3-relais-sop.national.inria.fr ([192.134.164.104]:36303
        "EHLO mail3-relais-sop.national.inria.fr" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S1726675AbfANQTx (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 14 Jan 2019 11:19:53 -0500
X-IronPort-AV: E=Sophos;i="5.56,478,1539640800"; 
   d="scan'208";a="291882267"
Received: from vaio-julia.rsr.lip6.fr ([132.227.76.33])
  by mail3-relais-sop.national.inria.fr with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Jan 2019 17:19:49 +0100
Date:   Mon, 14 Jan 2019 17:19:47 +0100 (CET)
From:   Julia Lawall <julia.lawall@lip6.fr>
X-X-Sender: jll@hadrien
To:     Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
cc:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        kernel-janitors@vger.kernel.org, David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] drm: rcar-du: add missing of_node_put
In-Reply-To: <78d7667d-f701-b908-9f24-0ee13be90947@ideasonboard.com>
Message-ID: <alpine.DEB.2.20.1901141719000.3522@hadrien>
References: <1547372691-28324-1-git-send-email-Julia.Lawall@lip6.fr> <1547372691-28324-2-git-send-email-Julia.Lawall@lip6.fr> <78d7667d-f701-b908-9f24-0ee13be90947@ideasonboard.com>
User-Agent: Alpine 2.20 (DEB 67 2015-01-07)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org



On Mon, 14 Jan 2019, Kieran Bingham wrote:

> Hi Julia,
>
> Thank you for the patch,
>
> On 13/01/2019 09:44, Julia Lawall wrote:
> > Add an of_node_put when the result of of_graph_get_remote_port_parent is
> > not available.
> >
> > The semantic match that finds this problem is as follows
> > (http://coccinelle.lip6.fr):
> >
> > // <smpl>
> > @r exists@
> > local idexpression e;
> > expression x;
> > @@
> > e = of_graph_get_remote_port_parent(...);
> > ... when != x = e
> >     when != true e == NULL
> >     when != of_node_put(e)
> >     when != of_fwnode_handle(e)
> > (
> > return e;
> > |
> > *return ...;
> > )
> > // </smpl>
> >
>
> For this part at least:
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> But a little discussion below:
>
>
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> This does indeed look like a missing of_node_put() but I don't think it
> will be the only one in that code. It gets a bit tangled - and I think
> there's possibly another missing put on the error path of
>
>         if (!encoder) {
>                 dev_warn(rcdu->dev,
>                          "no encoder found for endpoint %pOF, skipping\n",
>                          ep->local_node);
>                 return -ENODEV;
>         }
>
> because that implies that no encoder was found, and thus -
>                 encoder = entity;
>
> was not executed, and the tail of that function calls:
>
>          of_node_put(encoder);
>
> which would have been the final of_node_put(entity)

Looking at it again, I agree.  I can just extend this patch.

julia


>
>
> I think that part could be considered separate to this patch, but it's
> also quite closely related. Either way - I think it needs a second pair
> of eyes to see if I'm not crazy :)
>
> Regards
>
> Kieran
>
> >
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c |    1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > index 9c7007d..bc3fcb3 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > @@ -331,6 +331,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu,
> >  		dev_dbg(rcdu->dev,
> >  			"connected entity %pOF is disabled, skipping\n",
> >  			entity);
> > +		of_node_put(entity);
> >  		return -ENODEV;
> >  	}
> >
> >
>
>

From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julia Lawall <julia.lawall@lip6.fr>
Date: Mon, 14 Jan 2019 16:19:47 +0000
Subject: Re: [PATCH 1/3] drm: rcar-du: add missing of_node_put
Message-Id: <alpine.DEB.2.20.1901141719000.3522@hadrien>
List-Id: <kernel-janitors.vger.kernel.org>
References: <1547372691-28324-1-git-send-email-Julia.Lawall@lip6.fr>
 <1547372691-28324-2-git-send-email-Julia.Lawall@lip6.fr>
 <78d7667d-f701-b908-9f24-0ee13be90947@ideasonboard.com>
In-Reply-To: <78d7667d-f701-b908-9f24-0ee13be90947@ideasonboard.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: David Airlie <airlied@linux.ie>, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, Laurent Pinchart <laurent.pinchart@ideasonboard.com>



On Mon, 14 Jan 2019, Kieran Bingham wrote:

> Hi Julia,
>
> Thank you for the patch,
>
> On 13/01/2019 09:44, Julia Lawall wrote:
> > Add an of_node_put when the result of of_graph_get_remote_port_parent is
> > not available.
> >
> > The semantic match that finds this problem is as follows
> > (http://coccinelle.lip6.fr):
> >
> > // <smpl>
> > @r exists@
> > local idexpression e;
> > expression x;
> > @@
> > e = of_graph_get_remote_port_parent(...);
> > ... when != x = e
> >     when != true e = NULL
> >     when != of_node_put(e)
> >     when != of_fwnode_handle(e)
> > (
> > return e;
> > |
> > *return ...;
> > )
> > // </smpl>
> >
>
> For this part at least:
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> But a little discussion below:
>
>
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> This does indeed look like a missing of_node_put() but I don't think it
> will be the only one in that code. It gets a bit tangled - and I think
> there's possibly another missing put on the error path of
>
>         if (!encoder) {
>                 dev_warn(rcdu->dev,
>                          "no encoder found for endpoint %pOF, skipping\n",
>                          ep->local_node);
>                 return -ENODEV;
>         }
>
> because that implies that no encoder was found, and thus -
>                 encoder = entity;
>
> was not executed, and the tail of that function calls:
>
>          of_node_put(encoder);
>
> which would have been the final of_node_put(entity)

Looking at it again, I agree.  I can just extend this patch.

julia


>
>
> I think that part could be considered separate to this patch, but it's
> also quite closely related. Either way - I think it needs a second pair
> of eyes to see if I'm not crazy :)
>
> Regards
>
> Kieran
>
> >
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c |    1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > index 9c7007d..bc3fcb3 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > @@ -331,6 +331,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu,
> >  		dev_dbg(rcdu->dev,
> >  			"connected entity %pOF is disabled, skipping\n",
> >  			entity);
> > +		of_node_put(entity);
> >  		return -ENODEV;
> >  	}
> >
> >
>
>

From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH 1/3] drm: rcar-du: add missing of_node_put
Date: Mon, 14 Jan 2019 17:19:47 +0100 (CET)
Message-ID: <alpine.DEB.2.20.1901141719000.3522@hadrien>
References: <1547372691-28324-1-git-send-email-Julia.Lawall@lip6.fr>
 <1547372691-28324-2-git-send-email-Julia.Lawall@lip6.fr>
 <78d7667d-f701-b908-9f24-0ee13be90947@ideasonboard.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Return-path: <dri-devel-bounces@lists.freedesktop.org>
Received: from mail3-relais-sop.national.inria.fr
 (mail3-relais-sop.national.inria.fr [192.134.164.104])
 by gabe.freedesktop.org (Postfix) with ESMTPS id 4716F89E1B
 for <dri-devel@lists.freedesktop.org>; Mon, 14 Jan 2019 16:19:52 +0000 (UTC)
In-Reply-To: <78d7667d-f701-b908-9f24-0ee13be90947@ideasonboard.com>
List-Unsubscribe: <https://lists.freedesktop.org/mailman/options/dri-devel>,
 <mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <https://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <https://lists.freedesktop.org/mailman/listinfo/dri-devel>,
 <mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Errors-To: dri-devel-bounces@lists.freedesktop.org
Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>
To: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: David Airlie <airlied@linux.ie>, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, Laurent Pinchart <laurent.pinchart@ideasonboard.com>
List-Id: dri-devel@lists.freedesktop.org

CgpPbiBNb24sIDE0IEphbiAyMDE5LCBLaWVyYW4gQmluZ2hhbSB3cm90ZToKCj4gSGkgSnVsaWEs
Cj4KPiBUaGFuayB5b3UgZm9yIHRoZSBwYXRjaCwKPgo+IE9uIDEzLzAxLzIwMTkgMDk6NDQsIEp1
bGlhIExhd2FsbCB3cm90ZToKPiA+IEFkZCBhbiBvZl9ub2RlX3B1dCB3aGVuIHRoZSByZXN1bHQg
b2Ygb2ZfZ3JhcGhfZ2V0X3JlbW90ZV9wb3J0X3BhcmVudCBpcwo+ID4gbm90IGF2YWlsYWJsZS4K
PiA+Cj4gPiBUaGUgc2VtYW50aWMgbWF0Y2ggdGhhdCBmaW5kcyB0aGlzIHByb2JsZW0gaXMgYXMg
Zm9sbG93cwo+ID4gKGh0dHA6Ly9jb2NjaW5lbGxlLmxpcDYuZnIpOgo+ID4KPiA+IC8vIDxzbXBs
Pgo+ID4gQHIgZXhpc3RzQAo+ID4gbG9jYWwgaWRleHByZXNzaW9uIGU7Cj4gPiBleHByZXNzaW9u
IHg7Cj4gPiBAQAo+ID4gZSA9IG9mX2dyYXBoX2dldF9yZW1vdGVfcG9ydF9wYXJlbnQoLi4uKTsK
PiA+IC4uLiB3aGVuICE9IHggPSBlCj4gPiAgICAgd2hlbiAhPSB0cnVlIGUgPT0gTlVMTAo+ID4g
ICAgIHdoZW4gIT0gb2Zfbm9kZV9wdXQoZSkKPiA+ICAgICB3aGVuICE9IG9mX2Z3bm9kZV9oYW5k
bGUoZSkKPiA+ICgKPiA+IHJldHVybiBlOwo+ID4gfAo+ID4gKnJldHVybiAuLi47Cj4gPiApCj4g
PiAvLyA8L3NtcGw+Cj4gPgo+Cj4gRm9yIHRoaXMgcGFydCBhdCBsZWFzdDoKPgo+IFJldmlld2Vk
LWJ5OiBLaWVyYW4gQmluZ2hhbSA8a2llcmFuLmJpbmdoYW0rcmVuZXNhc0BpZGVhc29uYm9hcmQu
Y29tPgo+Cj4gQnV0IGEgbGl0dGxlIGRpc2N1c3Npb24gYmVsb3c6Cj4KPgo+ID4gU2lnbmVkLW9m
Zi1ieTogSnVsaWEgTGF3YWxsIDxKdWxpYS5MYXdhbGxAbGlwNi5mcj4KPgo+IFRoaXMgZG9lcyBp
bmRlZWQgbG9vayBsaWtlIGEgbWlzc2luZyBvZl9ub2RlX3B1dCgpIGJ1dCBJIGRvbid0IHRoaW5r
IGl0Cj4gd2lsbCBiZSB0aGUgb25seSBvbmUgaW4gdGhhdCBjb2RlLiBJdCBnZXRzIGEgYml0IHRh
bmdsZWQgLSBhbmQgSSB0aGluawo+IHRoZXJlJ3MgcG9zc2libHkgYW5vdGhlciBtaXNzaW5nIHB1
dCBvbiB0aGUgZXJyb3IgcGF0aCBvZgo+Cj4gICAgICAgICBpZiAoIWVuY29kZXIpIHsKPiAgICAg
ICAgICAgICAgICAgZGV2X3dhcm4ocmNkdS0+ZGV2LAo+ICAgICAgICAgICAgICAgICAgICAgICAg
ICAibm8gZW5jb2RlciBmb3VuZCBmb3IgZW5kcG9pbnQgJXBPRiwgc2tpcHBpbmdcbiIsCj4gICAg
ICAgICAgICAgICAgICAgICAgICAgIGVwLT5sb2NhbF9ub2RlKTsKPiAgICAgICAgICAgICAgICAg
cmV0dXJuIC1FTk9ERVY7Cj4gICAgICAgICB9Cj4KPiBiZWNhdXNlIHRoYXQgaW1wbGllcyB0aGF0
IG5vIGVuY29kZXIgd2FzIGZvdW5kLCBhbmQgdGh1cyAtCj4gICAgICAgICAgICAgICAgIGVuY29k
ZXIgPSBlbnRpdHk7Cj4KPiB3YXMgbm90IGV4ZWN1dGVkLCBhbmQgdGhlIHRhaWwgb2YgdGhhdCBm
dW5jdGlvbiBjYWxsczoKPgo+ICAgICAgICAgIG9mX25vZGVfcHV0KGVuY29kZXIpOwo+Cj4gd2hp
Y2ggd291bGQgaGF2ZSBiZWVuIHRoZSBmaW5hbCBvZl9ub2RlX3B1dChlbnRpdHkpCgpMb29raW5n
IGF0IGl0IGFnYWluLCBJIGFncmVlLiAgSSBjYW4ganVzdCBleHRlbmQgdGhpcyBwYXRjaC4KCmp1
bGlhCgoKPgo+Cj4gSSB0aGluayB0aGF0IHBhcnQgY291bGQgYmUgY29uc2lkZXJlZCBzZXBhcmF0
ZSB0byB0aGlzIHBhdGNoLCBidXQgaXQncwo+IGFsc28gcXVpdGUgY2xvc2VseSByZWxhdGVkLiBF
aXRoZXIgd2F5IC0gSSB0aGluayBpdCBuZWVkcyBhIHNlY29uZCBwYWlyCj4gb2YgZXllcyB0byBz
ZWUgaWYgSSdtIG5vdCBjcmF6eSA6KQo+Cj4gUmVnYXJkcwo+Cj4gS2llcmFuCj4KPiA+Cj4gPiAt
LS0KPiA+ICBkcml2ZXJzL2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1X2ttcy5jIHwgICAgMSArCj4g
PiAgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspCj4gPgo+ID4gZGlmZiAtLWdpdCBhL2Ry
aXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfa21zLmMgYi9kcml2ZXJzL2dwdS9kcm0vcmNh
ci1kdS9yY2FyX2R1X2ttcy5jCj4gPiBpbmRleCA5YzcwMDdkLi5iYzNmY2IzIDEwMDY0NAo+ID4g
LS0tIGEvZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9rbXMuYwo+ID4gKysrIGIvZHJp
dmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9rbXMuYwo+ID4gQEAgLTMzMSw2ICszMzEsNyBA
QCBzdGF0aWMgaW50IHJjYXJfZHVfZW5jb2RlcnNfaW5pdF9vbmUoc3RydWN0IHJjYXJfZHVfZGV2
aWNlICpyY2R1LAo+ID4gIAkJZGV2X2RiZyhyY2R1LT5kZXYsCj4gPiAgCQkJImNvbm5lY3RlZCBl
bnRpdHkgJXBPRiBpcyBkaXNhYmxlZCwgc2tpcHBpbmdcbiIsCj4gPiAgCQkJZW50aXR5KTsKPiA+
ICsJCW9mX25vZGVfcHV0KGVudGl0eSk7Cj4gPiAgCQlyZXR1cm4gLUVOT0RFVjsKPiA+ICAJfQo+
ID4KPiA+Cj4KPgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
XwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcK
aHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK