From patchwork Sat Feb 25 08:01:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Christophe JAILLET X-Patchwork-Id: 39583 X-Patchwork-Delegate: mchehab@kernel.org Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from ) id 1chXJE-0002yL-1n; Sat, 25 Feb 2017 08:02:28 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.84_2/mailfrontend-5) with esmtp id 1chXJB-0001sv-8P; Sat, 25 Feb 2017 09:02:27 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751709AbdBYIB4 (ORCPT + 1 other); Sat, 25 Feb 2017 03:01:56 -0500 Received: from smtp03.smtpout.orange.fr ([80.12.242.125]:17836 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750953AbdBYIBz (ORCPT ); Sat, 25 Feb 2017 03:01:55 -0500 Received: from [127.0.0.1] ([92.140.162.61]) by mwinf5d05 with ME id ow1K1u00d1KnKiL03w1L9c; Sat, 25 Feb 2017 09:01:24 +0100 X-ME-Helo: [127.0.0.1] X-ME-Date: Sat, 25 Feb 2017 09:01:24 +0100 X-ME-IP: 92.140.162.61 Subject: Re: [PATCH] [media] exynos4-is: Add missing 'of_node_put' To: Javier Martinez Canillas , kyungmin.park@samsung.com, s.nawrocki@samsung.com, mchehab@kernel.org, kgene@kernel.org, krzk@kernel.org References: <20170123211656.11185-1-christophe.jaillet@wanadoo.fr> <2357ef6e-8d90-77d0-0399-21fec41389a1@osg.samsung.com> Cc: linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Newsgroups: gmane.linux.kernel.janitors, gmane.linux.drivers.video-input-infrastructure, gmane.linux.ports.arm.kernel, gmane.linux.kernel.samsung-soc, gmane.linux.kernel From: Christophe JAILLET Message-ID: Date: Sat, 25 Feb 2017 09:01:19 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <2357ef6e-8d90-77d0-0399-21fec41389a1@osg.samsung.com> X-Antivirus: avast! (VPS 170224-1, 24/02/2017), Outbound message X-Antivirus-Status: Clean Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2017.2.25.75116 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, MSGID_ADDED_BY_MTA 0.05, BODY_SIZE_3000_3999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, IN_REP_TO 0, LEGITIMATE_SIGNS 0, MSG_THREAD 0, NO_URI_HTTPS 0, REFERENCES 0, SINGLE_URI_IN_BODY 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CP_URI_IN_BODY 0, __CT 0, __CTE 0, __CT_TEXT_PLAIN 0, __FORWARDED_MSG 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_LIST_ID 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_TEXT_P 0, __MIME_TEXT_P1 0, __MIME_VERSION 0, __MOZILLA_USER_AGENT 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_RCPTS_TO_X5 0, __NO_HTML_TAG_RAW 0, __PHISH_SPEAR_STRUCTURE_1 0, __REFERENCES 0, __SANE_MSGID 0, __SINGLE_URI_TEXT 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __TO_NAME 0, __TO_NAME_DIFF_FROM_ACC 0, __TO_REAL_NAMES 0, __URI_IN_BODY 0, __URI_NO_MAILTO 0, __URI_NO_WWW 0, __URI_NS , __URI_WITH_PATH 0, __USER_AGENT 0, __X_AV_AVAST 0' Le 24/02/2017 à 22:19, Javier Martinez Canillas a écrit : > Thanks for the patch, but Krzysztof sent the exact same patch before > [0]. There > was feedback from Sylwester at the time that you can also look at [0]. Could you > please take that into account and post a patch according to what he suggested? > > [0]: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/415207.html > > Best regards, Hi, apparently, the patch has already been taken into -next by Mauro Carvalho Chehab. Moreover, I personally don't think that what is proposed in [0] is more readable. There is not that much code between 'of_get_next_child' and 'of_node_put' in the normal path and not so many error handling paths between the 2 function calls. Adding some indentation level is not an improvement, IMHO. If you really want, I could propose something like the code below (not carefully checked, just to give an idea), but honestly, I don't think it is needed. It avoids some new indent and avoids duplicating 'of_node_put'. Tell me if you think it is an improvement and I'll send a patch. CJ pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_601; @@ -447,22 +444,28 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd, pd->fimc_bus_type = FIMC_BUS_TYPE_ISP_WRITEBACK; else pd->fimc_bus_type = pd->sensor_bus_type; if (WARN_ON(index >= ARRAY_SIZE(fmd->sensor))) { - of_node_put(rem); - return -EINVAL; + ret = -EINVAL; + goto put_rem; } fmd->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_OF; fmd->sensor[index].asd.match.of.node = rem; fmd->async_subdevs[index] = &fmd->sensor[index].asd; fmd->num_sensors++; +out: + ret = 0; +put_rem: of_node_put(rem); - return 0; + +put_ep: + of_node_put(ep); + return ret; } /* Register all SoC external sub-devices */ static int fimc_md_register_sensor_entities(struct fimc_md *fmd) { diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index e3a8709138fa..da5b76c1df98 100644 --- a/drivers/media/platform/exynos4-is/media-dev.c +++ b/drivers/media/platform/exynos4-is/media-dev.c @@ -385,38 +385,35 @@ static void fimc_md_pipelines_free(struct fimc_md *fmd) static int fimc_md_parse_port_node(struct fimc_md *fmd, struct device_node *port, unsigned int index) { struct fimc_source_info *pd = &fmd->sensor[index].pdata; - struct device_node *rem, *ep, *np; + struct device_node *rem = NULL, *ep = NULL, *np; struct v4l2_of_endpoint endpoint; int ret; /* Assume here a port node can have only one endpoint node. */ ep = of_get_next_child(port, NULL); if (!ep) return 0; ret = v4l2_of_parse_endpoint(ep, &endpoint); - if (ret) { - of_node_put(ep); - return ret; - } + if (ret) + goto put_ep; if (WARN_ON(endpoint.base.port == 0) || index >= FIMC_MAX_SENSORS) { - of_node_put(ep); - return -EINVAL; + ret = -EINVAL; + goto put_ep; } pd->mux_id = (endpoint.base.port - 1) & 0x1; rem = of_graph_get_remote_port_parent(ep); - of_node_put(ep); if (rem == NULL) { v4l2_info(&fmd->v4l2_dev, "Remote device at %s not found\n", ep->full_name); - return 0; + goto out; } if (fimc_input_is_parallel(endpoint.base.port)) { if (endpoint.bus_type == V4L2_MBUS_PARALLEL)