[XEN PATCH v2] x86/iommu_init: address a violation of MISRA C:2012 Rule 8.3

Federico Serafini posted 1 patch 6 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/ba5d1368fce181a6a3a6abc150651e1e5323e489.1698238686.git.federico.serafini@bugseng.com
xen/drivers/passthrough/amd/iommu_init.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[XEN PATCH v2] x86/iommu_init: address a violation of MISRA C:2012 Rule 8.3
Posted by Federico Serafini 6 months, 1 week ago
Make function definition and declaration consistent and emphasize that
the formal parameter is deliberately not used.
No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
Changes in v2:
- improved code format.
---
 xen/drivers/passthrough/amd/iommu_init.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 9c01a49435..9f955743e1 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -692,7 +692,7 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu)
     spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
-static void cf_check do_amd_iommu_irq(void *unused)
+static void cf_check do_amd_iommu_irq(void *data)
 {
     struct amd_iommu *iommu;
 
@@ -702,6 +702,9 @@ static void cf_check do_amd_iommu_irq(void *unused)
         return;
     }
 
+    /* Formal parameter is deliberately unused. */
+    (void)data;
+
     /*
      * No matter from where the interrupt came from, check all the
      * IOMMUs present in the system. This allows for having just one
-- 
2.34.1
Re: [XEN PATCH v2] x86/iommu_init: address a violation of MISRA C:2012 Rule 8.3
Posted by Jan Beulich 6 months ago
On 25.10.2023 15:01, Federico Serafini wrote:
> Make function definition and declaration consistent and emphasize that
> the formal parameter is deliberately not used.

Coming back to my earlier objection: Did you consider alternatives? Best
would of course be to get rid of the forward declaration. That seems
possible, albeit not quite as straightforward as it ended up being in
other cases. Second best would be to rename the parameter in the forward
declaration. Question of course in how far "emphasize that the formal
parameter is deliberately not used" is important here. (If it was, I
wonder why VT-d's do_iommu_page_fault() is left alone.)

Jan

> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -692,7 +692,7 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu)
>      spin_unlock_irqrestore(&iommu->lock, flags);
>  }
>  
> -static void cf_check do_amd_iommu_irq(void *unused)
> +static void cf_check do_amd_iommu_irq(void *data)
>  {
>      struct amd_iommu *iommu;
>  
> @@ -702,6 +702,9 @@ static void cf_check do_amd_iommu_irq(void *unused)
>          return;
>      }
>  
> +    /* Formal parameter is deliberately unused. */
> +    (void)data;
> +
>      /*
>       * No matter from where the interrupt came from, check all the
>       * IOMMUs present in the system. This allows for having just one
Re: [XEN PATCH v2] x86/iommu_init: address a violation of MISRA C:2012 Rule 8.3
Posted by Federico Serafini 6 months ago
On 30/10/23 16:01, Jan Beulich wrote:
> On 25.10.2023 15:01, Federico Serafini wrote:
>> Make function definition and declaration consistent and emphasize that
>> the formal parameter is deliberately not used.
> 
> Coming back to my earlier objection: Did you consider alternatives? Best
> would of course be to get rid of the forward declaration. That seems
> possible, albeit not quite as straightforward as it ended up being in
> other cases. Second best would be to rename the parameter in the forward
> declaration. Question of course in how far "emphasize that the formal
> parameter is deliberately not used" is important here. (If it was, I
> wonder why VT-d's do_iommu_page_fault() is left alone.)
> 
> Jan

I can propose a new version of the patch with the second option.
If one day you will decide to accept also Rule 2.7 ("A function
should not contain unused parameters"), then a deviation based on
the parameter name "unused" would be viable.

If, however, there is interest in applying the first option,
I think the best thing is for you to take care of it.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)