[igt-dev] [PATCH i-g-t v2 1/1] lib/igt_device_scan: clear igt_devs.devs_scanned in igt_devices_free

[Tauro, Riana]

On 9/29/2022 3:53 PM, Janusz Krzysztofik wrote:
> On Thursday, 29 September 2022 11:47:53 CEST Tauro, Riana wrote:
>>
>> On 9/29/2022 2:31 PM, Janusz Krzysztofik wrote:
>>> On Thursday, 29 September 2022 06:50:58 CEST Riana Tauro wrote:
>>>> igt_devices_scan returns with empty list if devs_scanned
>>>> is true and function is called after igt_devices_free
>>>>
>>>> clear igt_devs.devs_scanned in igt_devices_free.
>>>>
>>>> v2: optimize code in igt_devices_scan (Zbigniew)
>>>>
>>>> Signed-off-by: Riana Tauro <riana.tauro at intel.com>
>>>> ---
>>>>    lib/igt_device_scan.c | 20 ++++----------------
>>>>    1 file changed, 4 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
>>>> index eb6b45b8..8b70e375 100644
>>>> --- a/lib/igt_device_scan.c
>>>> +++ b/lib/igt_device_scan.c
>>>> @@ -1027,9 +1027,11 @@ void igt_devices_free(void)
>>>>    	}
>>>>    
>>>>    	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
>>>> +		igt_list_del(&dev->link);
>>>>    		igt_device_free(dev);
>>>>    		free(dev);
>>>>    	}
>>>> +	igt_devs.devs_scanned = false;
>>>>    }
>>>>    
>>>>    /**
>>>> @@ -1043,22 +1045,8 @@ void igt_devices_free(void)
>>>>     */
>>>>    void igt_devices_scan(bool force)
>>>>    {
>>>> -	if (force && igt_devs.devs_scanned) {
>>>> -		struct igt_device *dev, *tmp;
>>>> -
>>>> -		igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered,
>>>> -					     link) {
>>>> -			igt_list_del(&dev->link);
>>>> -			free(dev);
>>>> -		}
>>>
>>> Why don't we need to clean the igt_devs.filtered list up any longer?
>>>
>>> Thanks,
>>> Janusz
> 
>> This patch is removing redundant code
>>
>> igt_devices_free() has the clean up of igt_devs.filtered list and
>> igt_devs.all. Using igt_devices_free() to remove redundancy
> 
> I see, but then, it would be more clear for everyone if you didn't mix that
> optimisation with the fix of broken igt_devices_free() in one patch, unless
> you mention explicitly (not only via v2 update) in commit description that the
> patch covers two topics: the fix, and the optimisation.
> 
> Thanks,
> Janusz

Sorry for the confusion, will fix the commit message.

Thanks
Riana
>>
>> Thanks
>> Riana
>>>
>>>> -		igt_list_for_each_entry_safe(dev, tmp, &igt_devs.all, link) {
>>>> -			igt_list_del(&dev->link);
>>>> -			igt_device_free(dev);
>>>> -			free(dev);
>>>> -		}
>>>> -
>>>> -		igt_devs.devs_scanned = false;
>>>> -	}
>>>> +	if (force && igt_devs.devs_scanned)
>>>> +		igt_devices_free();
>>>>    
>>>>    	if (igt_devs.devs_scanned)
>>>>    		return;
>>>>
>>>
>>>
>>>
>>>
>>
> 
> 
> 
>