Re: [PATCH v2 1/8] driver/pci: Get next capability without passing caps

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

On 2025/4/11 16:00, Jan Beulich wrote:
> On 11.04.2025 04:51, Chen, Jiqian wrote:
>> On 2025/4/10 20:34, Jan Beulich wrote:
>>> On 09.04.2025 08:45, Jiqian Chen wrote:
>>>> --- a/xen/drivers/pci/pci.c
>>>> +++ b/xen/drivers/pci/pci.c
>>>> @@ -40,7 +40,7 @@ unsigned int pci_find_cap_offset(pci_sbdf_t sbdf, 
>>>> unsigned int cap)
>>>>  }
>>>>  
>>>>  unsigned int pci_find_next_cap_ttl(pci_sbdf_t sbdf, unsigned int pos,
>>>> -                                   const unsigned int caps[], unsigned 
>>>> int n,
>>>> +                                   const unsigned int *caps, unsigned int 
>>>> n,
>>>
>>> I don't follow the need for this change.
>> This changed is for my next patch "vpci/header: Emulate legacy capability 
>> list for host".
>> Currently, vpci only emulates capability list for domU, not for dom0.
>> For domU, vpci exposes a fixed capability array which calls "supported_caps".
>> My changes want to emulate capability list for dom0.
>> I think vpci should expose all devices capabilities to dom0.
>> When I emulate it, I need to iterate over all capabilities without another 
>> fixed array,
>> so I need this function to return the position of next capability directly 
>> when passing a zero length array to this function.
> 
> This doesn't answer my question. The change you need for the next patch is
> the hunk below, not the one above. Aiui at least.
Ah, yes.
I will abandon changing this function definition in next version since it just 
needs to check "n == 0".

> 
>>>> @@ -55,6 +55,10 @@ unsigned int pci_find_next_cap_ttl(pci_sbdf_t sbdf, 
>>>> unsigned int pos,
>>>>  
>>>>          if ( id == 0xff )
>>>>              break;
>>>> +
>>>> +        if ( !caps || n == 0 )
>>>> +            return pos;
>>>
>>> Checking n to be zero ought to suffice here? In that case it doesn't matter
>>> what caps is. Plus if n is non-zero, it clearly is an error if caps was 
>>> NULL.
>> Two checking is to prevent null pointer errors.
>> But as you said, if checking n to be zero is enough, then I don't need to 
>> change the definition of this function.
>> I will change in next version.
> 
> If you really wanted to, you could add e.g. ASSERT(caps) after this if().
OK, will add in next version, too.

> 
> Jan

-- 
Best regards,
Jiqian Chen.