Re: [PATCH i-g-t v1 3/4] lib/igt_sriov_device: Add helper to iterate over VFs from starting number

["Laguna, Lukasz" <lukasz.laguna@intel.com>]

On 1/27/2025 10:06, Michal Wajdeczko wrote:
>
> On 24.01.2025 09:10, Lukasz Laguna wrote:
>> Helper allows to iterate over VFs associated with given PF, starting
>> from a specified VF number.
>>
>> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
>> ---
>>   lib/igt_sriov_device.h | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/lib/igt_sriov_device.h b/lib/igt_sriov_device.h
>> index de25a7d98..741b1bcec 100644
>> --- a/lib/igt_sriov_device.h
>> +++ b/lib/igt_sriov_device.h
>> @@ -86,6 +86,21 @@ igt_sriov_random_vf_in_range(int pf_fd, unsigned int start, unsigned int end)
>>   	     ++__vf_num)
>>   #define for_each_sriov_num_vfs for_each_sriov_vf
>>   
>> +/**
>> + * for_each_sriov_vf_starting_from - Iterate over VFs starting from a specified VF
>> + * @__pf_fd: PF device file descriptor
>> + * @__start: Starting VF number
>> + * @__vf_num: VFs iterator
>> + *
>> + * For loop that iterates over VFs associated with given PF @__pf_fd,
>> + * starting from a specified VF number @__start.
>> + */
>> +#define for_each_sriov_vf_starting_from(__pf_fd, __start, __vf_num) \
> nit: there is no need to use underscore for the macro params as all will
> be expanded/replaced as-is
>
>> +	for (unsigned int __vf_num = __start, __total_vfs = igt_sriov_get_total_vfs(__pf_fd); \
> macro param "start" shall be wrapped in ( )
>
>> +	     __vf_num <= __total_vfs; \
>> +	     ++__vf_num)
> macro param "vf_num" shall be wrapped in ( )
>
> and there is a risk of param re-evaluation due to param reuse
> maybe just add new local temp var for it?

The macro ensures that __vf_num is treated as a variable name and not an 
expression. I don't see a risk of param re-evaluation in this case.

>
>> +#define for_each_sriov_num_vfs_starting_from for_each_sriov_vf_starting_from
> why do we need another alias for this new macro?
> if the original name is wrong then just use a better one ;)

Depending on scenario, we want to be clear that we use number of VFs or 
VF number.

>> +
>>   /**
>>    * for_random_sriov_vf_in_range - Iterate over a random VF in a specified range
>>    * @__pf_fd: PF device file descriptor