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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7677FEB64DC for ; Mon, 17 Jul 2023 07:42:42 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6A311865E4; Mon, 17 Jul 2023 09:42:40 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.b="KevVHr+M"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="zn+ivXck"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CAD21865E4; Mon, 17 Jul 2023 09:42:38 +0200 (CEST) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id B1F398639C for ; Mon, 17 Jul 2023 09:42:35 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=msuchanek@suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 7408E1F74C; Mon, 17 Jul 2023 07:42:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1689579755; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RzLaGes2luj6jfgNFTo4vkx/691S0aBJG1Uhhag98Hk=; b=KevVHr+MkYd5oskPXKzA4DhiN+y2mON7LVblaZ9vwVmNZ9y0NsJCudk4M1IkjMu/XpyPBK gEtSiHg9chmC7ujZBAoCv23Txjy27CodkitHEsjr7gOksYgmb7nXBRCsuBX7J7yaIbInUf lOtc//9/Ck9sp00McSpWAKxfIb5pIlc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1689579755; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RzLaGes2luj6jfgNFTo4vkx/691S0aBJG1Uhhag98Hk=; b=zn+ivXckMYobJpAzeS21m/SUr9xFUFhcsbzaLe8I2nmr9TDPqGMzd1jarQLJDRiWLbOlLQ l+X2w4Xzhlr5KyCg== Received: from kitsune.suse.cz (kitsune.suse.cz [10.100.12.127]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 3A6102C142; Mon, 17 Jul 2023 07:42:35 +0000 (UTC) Date: Mon, 17 Jul 2023 09:42:34 +0200 From: Michal =?iso-8859-1?Q?Such=E1nek?= To: Marek Vasut Cc: u-boot@lists.denx.de, Jonas Karlman , Pali =?iso-8859-1?Q?Roh=E1r?= , Bin Meng , Simon Glass Subject: Re: [PATCH] pci: Fix device_find_first_child() return value handling Message-ID: <20230717074233.GP9196@kitsune.suse.cz> References: <20230716155324.11211-1-marex@denx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230716155324.11211-1-marex@denx.de> User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean Hello, On Sun, Jul 16, 2023 at 05:53:24PM +0200, Marek Vasut wrote: > This function only ever returns 0, but may not assign the second > parameter. Same thing for device_find_next_child(). Do not assign > ret to stop proliferation of this misuse. > > Reported-by: Jonas Karlman > Signed-off-by: Marek Vasut > --- > Cc: "Pali Rohár" > Cc: Bin Meng > Cc: Marek Vasut > Cc: Michal Suchanek > Cc: Simon Glass > --- > drivers/pci/pci-uclass.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c > index 8d27e40338c..6421eda7721 100644 > --- a/drivers/pci/pci-uclass.c > +++ b/drivers/pci/pci-uclass.c > @@ -545,9 +545,9 @@ int pci_auto_config_devices(struct udevice *bus) > sub_bus = dev_seq(bus); > debug("%s: start\n", __func__); > pciauto_config_init(hose); > - for (ret = device_find_first_child(bus, &dev); > - !ret && dev; > - ret = device_find_next_child(&dev)) { > + for (device_find_first_child(bus, &dev); > + dev; > + device_find_next_child(&dev)) { Sounds like you will need to remove the declaration of the now unused ret variable as well. More generally, what is the overall vision for these functions returning always zero? Should the return value be kept in case the underlying implementation changes and errors can happen in the future, and consequently checked? Should the return value be removed when meaningless making these useless assignments and checks an error? I already elimimnated a return value where using it lead to incorrect behavior but here using it or not is equally correct with the current implementation. Thanks Michal