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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

>>> @@ -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().

Jan