[PATCH] pci: Fix device_find_first_child() return value handling

[Michal Suchánek]

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 <jonas at kwiboo.se>
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: "Pali Rohár" <pali at kernel.org>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> Cc: Marek Vasut <marex at denx.de>
> Cc: Michal Suchanek <msuchanek at suse.de>
> Cc: Simon Glass <sjg at chromium.org>
> ---
>  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