diff mbox series

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

Message ID ba5d1368fce181a6a3a6abc150651e1e5323e489.1698238686.git.federico.serafini@bugseng.com (mailing list archive)
State New, archived
Headers show
Series [XEN,v2] x86/iommu_init: address a violation of MISRA C:2012 Rule 8.3 | expand

Commit Message

Federico Serafini Oct. 25, 2023, 1:01 p.m. UTC
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(-)

Comments

Jan Beulich Oct. 30, 2023, 3:01 p.m. UTC | #1
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
Federico Serafini Nov. 2, 2023, 10:17 a.m. UTC | #2
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.
Jan Beulich Nov. 2, 2023, 2:24 p.m. UTC | #3
On 02.11.2023 11:17, Federico Serafini wrote:
> 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.)
> 
> 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.

Done.

Jan
diff mbox series

Patch

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