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

[Marek Vasut]

On 7/17/23 09:42, Michal Suchánek wrote:
> 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.

Probably a question for Simon, really. Personally I would be tempted to 
switch the function to return void.