[XEN PATCH] x86: I/O emulation: fix violations of MISRA C:2012 Rules 8.2 and 8.3

Federico Serafini posted 1 patch 9 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/b6ebf3a49de027981505da63aef594cb0dc42ead.1689691260.git.federico.serafini@bugseng.com
xen/arch/x86/include/asm/hvm/emulate.h |  8 ++++----
xen/arch/x86/include/asm/hvm/io.h      | 14 +++++++-------
2 files changed, 11 insertions(+), 11 deletions(-)
[XEN PATCH] x86: I/O emulation: fix violations of MISRA C:2012 Rules 8.2 and 8.3
Posted by Federico Serafini 9 months, 2 weeks ago
Give a name to unnamed parameters thus fixing violations of
MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with
named parameters").
Keep consistency between parameter names used in function declarations
and names used in the corresponding function definitions thus fixing
violations of MISRA C:2012 Rule 8.3 ("All declarations of an object or
function shall use the same names and type qualifiers").

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/x86/include/asm/hvm/emulate.h |  8 ++++----
 xen/arch/x86/include/asm/hvm/io.h      | 14 +++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/include/asm/hvm/emulate.h b/xen/arch/x86/include/asm/hvm/emulate.h
index d8ba2df4e4..398d0db078 100644
--- a/xen/arch/x86/include/asm/hvm/emulate.h
+++ b/xen/arch/x86/include/asm/hvm/emulate.h
@@ -121,12 +121,12 @@ static inline void hvmemul_cache_destroy(struct vcpu *v)
 {
     XFREE(v->arch.hvm.hvm_io.cache);
 }
-bool hvmemul_read_cache(const struct vcpu *, paddr_t gpa,
+bool hvmemul_read_cache(const struct vcpu *v, paddr_t gpa,
                         void *buffer, unsigned int size);
-void hvmemul_write_cache(const struct vcpu *, paddr_t gpa,
+void hvmemul_write_cache(const struct vcpu *v, paddr_t gpa,
                          const void *buffer, unsigned int size);
-unsigned int hvmemul_cache_disable(struct vcpu *);
-void hvmemul_cache_restore(struct vcpu *, unsigned int token);
+unsigned int hvmemul_cache_disable(struct vcpu *v);
+void hvmemul_cache_restore(struct vcpu *v, unsigned int token);
 /* For use in ASSERT()s only: */
 static inline bool hvmemul_cache_disabled(struct vcpu *v)
 {
diff --git a/xen/arch/x86/include/asm/hvm/io.h b/xen/arch/x86/include/asm/hvm/io.h
index 8df33eb6cc..e5225e75ef 100644
--- a/xen/arch/x86/include/asm/hvm/io.h
+++ b/xen/arch/x86/include/asm/hvm/io.h
@@ -46,17 +46,17 @@ struct hvm_io_handler {
     uint8_t type;
 };
 
-typedef int (*hvm_io_read_t)(const struct hvm_io_handler *,
+typedef int (*hvm_io_read_t)(const struct hvm_io_handler *handler,
                              uint64_t addr,
                              uint32_t size,
                              uint64_t *data);
-typedef int (*hvm_io_write_t)(const struct hvm_io_handler *,
+typedef int (*hvm_io_write_t)(const struct hvm_io_handler *handler,
                               uint64_t addr,
                               uint32_t size,
                               uint64_t data);
-typedef bool_t (*hvm_io_accept_t)(const struct hvm_io_handler *,
+typedef bool_t (*hvm_io_accept_t)(const struct hvm_io_handler *handler,
                                   const ioreq_t *p);
-typedef void (*hvm_io_complete_t)(const struct hvm_io_handler *);
+typedef void (*hvm_io_complete_t)(const struct hvm_io_handler *handler);
 
 struct hvm_io_ops {
     hvm_io_accept_t   accept;
@@ -87,11 +87,11 @@ bool relocate_portio_handler(
 
 void send_timeoffset_req(unsigned long timeoff);
 bool handle_mmio_with_translation(unsigned long gla, unsigned long gpfn,
-                                  struct npfec);
+                                  struct npfec access);
 bool handle_pio(uint16_t port, unsigned int size, int dir);
 void hvm_interrupt_post(struct vcpu *v, int vector, int type);
-void hvm_dpci_eoi(struct domain *d, unsigned int guest_irq);
-void msix_write_completion(struct vcpu *);
+void hvm_dpci_eoi(struct domain *d, unsigned int guest_gsi);
+void msix_write_completion(struct vcpu *v);
 
 #ifdef CONFIG_HVM
 void msixtbl_init(struct domain *d);
-- 
2.34.1
Re: [XEN PATCH] x86: I/O emulation: fix violations of MISRA C:2012 Rules 8.2 and 8.3
Posted by Paul Durrant 9 months, 2 weeks ago
On 19/07/2023 09:24, Federico Serafini wrote:
> Give a name to unnamed parameters thus fixing violations of
> MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with
> named parameters").
> Keep consistency between parameter names used in function declarations
> and names used in the corresponding function definitions thus fixing
> violations of MISRA C:2012 Rule 8.3 ("All declarations of an object or
> function shall use the same names and type qualifiers").
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
>   xen/arch/x86/include/asm/hvm/emulate.h |  8 ++++----
>   xen/arch/x86/include/asm/hvm/io.h      | 14 +++++++-------
>   2 files changed, 11 insertions(+), 11 deletions(-)
> 

Reviewed-by: Paul Durrant <paul@xen.org>
Re: [XEN PATCH] x86: I/O emulation: fix violations of MISRA C:2012 Rules 8.2 and 8.3
Posted by Jan Beulich 9 months, 2 weeks ago
On 19.07.2023 10:24, Federico Serafini wrote:
> Give a name to unnamed parameters thus fixing violations of
> MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with
> named parameters").
> Keep consistency between parameter names used in function declarations
> and names used in the corresponding function definitions thus fixing
> violations of MISRA C:2012 Rule 8.3 ("All declarations of an object or
> function shall use the same names and type qualifiers").
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
>  xen/arch/x86/include/asm/hvm/emulate.h |  8 ++++----
>  xen/arch/x86/include/asm/hvm/io.h      | 14 +++++++-------
>  2 files changed, 11 insertions(+), 11 deletions(-)

If it was just the 2nd file, I'd agree with the "I/O" in the title
(albeit as a minor remark and as mentioned elsewhere, personally I
think double tags in titles are undesirable, and blanks in tags
aren't nice either). So perhaps "x86/HVM/emul:" ?

The code changes themselves look okay to me (no matter that I don't
like some of them), so
Acked-by: Jan Beulich <jbeulich@suse.com>
I'd be happy to make, while committing, whatever title adjustment
that you can agree with.

One other note though (there's no good general place to put it):
I'm also a little unhappy with all of you often using "fix" in the
titles, when you don't really fix any bugs. There are certainly
cases where addressing Misra complaints also fixes bugs, but that's
more the exception than the rule. Could we settle on something
like "eliminate", "address", "avoid", or alike when changes are
merely about style or other aspect which don't really correct
functionality?

Jan
Re: [XEN PATCH] x86: I/O emulation: fix violations of MISRA C:2012 Rules 8.2 and 8.3
Posted by Federico Serafini 9 months, 2 weeks ago
On 19/07/23 10:32, Jan Beulich wrote:
> On 19.07.2023 10:24, Federico Serafini wrote:
>> Give a name to unnamed parameters thus fixing violations of
>> MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with
>> named parameters").
>> Keep consistency between parameter names used in function declarations
>> and names used in the corresponding function definitions thus fixing
>> violations of MISRA C:2012 Rule 8.3 ("All declarations of an object or
>> function shall use the same names and type qualifiers").
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>> ---
>>   xen/arch/x86/include/asm/hvm/emulate.h |  8 ++++----
>>   xen/arch/x86/include/asm/hvm/io.h      | 14 +++++++-------
>>   2 files changed, 11 insertions(+), 11 deletions(-)
> 
> If it was just the 2nd file, I'd agree with the "I/O" in the title
> (albeit as a minor remark and as mentioned elsewhere, personally I
> think double tags in titles are undesirable, and blanks in tags
> aren't nice either). So perhaps "x86/HVM/emul:" ?

"x86/HVM/emul:" is okay.
I will follow your suggestions in the next patches.

> The code changes themselves look okay to me (no matter that I don't
> like some of them), so
> Acked-by: Jan Beulich <jbeulich@suse.com>
> I'd be happy to make, while committing, whatever title adjustment
> that you can agree with.
> 
> One other note though (there's no good general place to put it):
> I'm also a little unhappy with all of you often using "fix" in the
> titles, when you don't really fix any bugs. There are certainly
> cases where addressing Misra complaints also fixes bugs, but that's
> more the exception than the rule. Could we settle on something
> like "eliminate", "address", "avoid", or alike when changes are
> merely about style or other aspect which don't really correct
> functionality?
> 
> Jan

Sure, "address" seems good to us.
Please, while committing, change the occurrences of "fix" into "address"
for the commit title and and commit description as well
(together with the tag change).

Regards
-- 
Federico Serafini, M.Sc.

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