Re: [XEN PATCH] amd/iommu: add fixed size to function parameter of array type

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.03.2024 10:25, Nicola Vetrini wrote:
> On 2024-03-14 09:32, Jan Beulich wrote:
>> On 14.03.2024 08:42, Nicola Vetrini wrote:
>>> The 'cmd' parameter of amd_iommu_send_guest_cmd is passed
>>> to a function that expects arrays of size 4, therefore
>>> specifying explicitly the size also in amd_iommu_send_guest_cmd
>>> allows not to accidentally pass a smaller array or assume that
>>> send_iommu_command handles array sizes >4 correctly.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
>>> ---
>>> All current users pass an array of size 4, hence this patch is 
>>> addressing
>>> a potential issue noticed by the analyzer in the context of Rule 17.5
>>> ("The function argument corresponding to a parameter declared to have 
>>> an array
>>> type shall have an appropriate number of elements"), not an actual 
>>> problem in
>>> the sources.
>>
>> While true, I think we want to consider alternatives. First one being 
>> to rip
>> out this dead code (thus addressing other Misra concerns as well). 
>> Second,
>> if we meant to keep it, to properly do away with the (ab)use of u32[].
>>
> 
> I'm not understanding what you consider dead code.

iommu_guest.c:guest_iommu_{init,destroy,set_base,add_event_log}() have
no callers; guest_iommu_add_ppr_log() having one is suspicious, but the
function will still bail early due to domain_iommu() never returning
non-NULL in practice. With that I'm pretty sure nothing else in the file
is actually reachable.

> I see three users of amd_iommu_send_guest_cmd

All in said file.

> and seven for send_iommu_command.

Well, this one of course isn't dead.

> I can adjust u32 for sure. There are also other u32/uint32_t 
> incosistencies in that header.

Which we're going to take care of over time.

Jan