From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CD321C001B0 for ; Mon, 17 Jul 2023 19:32:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231550AbjGQTcZ (ORCPT ); Mon, 17 Jul 2023 15:32:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34790 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231515AbjGQTcW (ORCPT ); Mon, 17 Jul 2023 15:32:22 -0400 Received: from smtp.smtpout.orange.fr (smtp-15.smtpout.orange.fr [80.12.242.15]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 61B2F9D for ; Mon, 17 Jul 2023 12:32:18 -0700 (PDT) Received: from [192.168.1.18] ([86.243.2.178]) by smtp.orange.fr with ESMTPA id LTx9qUbpG8AXRLTx9qnrOu; Mon, 17 Jul 2023 21:32:16 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wanadoo.fr; s=t20230301; t=1689622336; bh=fgVFvsM19zb6DaIaPAT/vgxgFwW4LN9BClFowleWbAo=; h=Date:Subject:To:Cc:References:From:In-Reply-To; b=QYWPLNrznyX3OwoF7j4CiAhkK5mHrVbhSjZsQRtaKsxkeYoG9aVLpEdpGUCck4ANy EM3qn7fLWJOYcP02W8t6bw7NCo/UJwxUzwOsTGiGexoqypRTchYBGSk5KKbt5L1Wfx n5rGf8jWAH41olni/e8TP6iUyWcCI7edd6jW5a6iUtoi1QxIMCKBanbsUHhr9J+mFK RArhbAk2KjY2HEd0lmIGaFPCjDeIz3UGAuKm7MmJ+3n6o1378/k9X/oPRVfC44q7xq eLjDG8yvMUttRLdrjfw8IEDgIqAA6n/vmd0mFOrBkuDp973uAwx84XwTkk/Dojjy4J 3Rf3R+C3d56dA== X-ME-Helo: [192.168.1.18] X-ME-Auth: Y2hyaXN0b3BoZS5qYWlsbGV0QHdhbmFkb28uZnI= X-ME-Date: Mon, 17 Jul 2023 21:32:16 +0200 X-ME-IP: 86.243.2.178 Message-ID: <5c10619b-14cb-a765-488c-8fbda857b4c2@wanadoo.fr> Date: Mon, 17 Jul 2023 21:32:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH] driver core: Fix an error handling path in fw_devlink_create_devlink() Content-Language: fr, en-US To: Saravana Kannan Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org References: <858b140b276ceab52a84ce0d4f924b0c411560d1.1689618373.git.christophe.jaillet@wanadoo.fr> From: Christophe JAILLET In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 17/07/2023 à 20:57, Saravana Kannan a écrit : > On Mon, Jul 17, 2023 at 11:26 AM Christophe JAILLET > wrote: >> >> All error handling paths go to 'out', except this one. Be consistent and >> also branch to 'out' here. >> >> Fixes: 74c782cff77b ("driver core: fw_devlink: Handle suppliers that don't use driver core") >> Signed-off-by: Christophe JAILLET >> --- >> /!\ Speculative /!\ >> >> This patch is based on analysis of the surrounding code and should be >> reviewed with care ! >> >> If the patch is wrong, maybe a comment in the code could explain why. >> >> /!\ Speculative /!\ > > Looks okay to me, but I'd rather not much around the code for a > subjective preference. The existing code also avoids the pointless > put_device() and personally is easier to read (ok, nothing more of > relevance is done after this return) than trying to follow the > execution to the out and then realizing the put_device() is a NOP, > etc. It also means future changes to "out" will need to accommodate > for this path that doesn't really need the "out" path. > > -Saravana Agreed, I didn't realized that this code was executed only if "if (sup_dev)" fails, so only if sup_dev is NULL. Sorry for the noise. CJ > >> --- >> drivers/base/core.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/base/core.c b/drivers/base/core.c >> index 3dff5037943e..854c1fab742c 100644 >> --- a/drivers/base/core.c >> +++ b/drivers/base/core.c >> @@ -2107,7 +2107,8 @@ static int fw_devlink_create_devlink(struct device *con, >> fwnode_ancestor_init_without_drv(sup_handle)) { >> dev_dbg(con, "Not linking %pfwf - might never become dev\n", >> sup_handle); >> - return -EINVAL; >> + ret = -EINVAL; >> + goto out; >> } >> >> ret = -EAGAIN; >> -- >> 2.34.1 >> >