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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 14 Mar 2024, Jan Beulich wrote:
> 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 also prefer removing the code that is not used


> > 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
>