[PATCH 2/5] x86: Name parameters in function declarations

Andrew Cooper posted 5 patches 2 days, 1 hour ago
[PATCH 2/5] x86: Name parameters in function declarations
Posted by Andrew Cooper 2 days, 1 hour ago
With the wider testing, some more violations have been spotted.  This
addresses violations of Rule 8.2 (parameters must be named).

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: consulting@bugseng.com <consulting@bugseng.com>
CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/mm/shadow/common.c | 8 ++++----
 xen/arch/x86/pv/emul-priv-op.c  | 2 +-
 xen/include/xen/livepatch.h     | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 423764a32653..f2aee5be46a7 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -69,11 +69,11 @@ const uint8_t sh_type_to_size[] = {
 
 DEFINE_PER_CPU(uint32_t,trace_shadow_path_flags);
 
-static int cf_check sh_enable_log_dirty(struct domain *);
-static int cf_check sh_disable_log_dirty(struct domain *);
-static void cf_check sh_clean_dirty_bitmap(struct domain *);
+static int cf_check sh_enable_log_dirty(struct domain *d);
+static int cf_check sh_disable_log_dirty(struct domain *d);
+static void cf_check sh_clean_dirty_bitmap(struct domain *d);
 
-static void cf_check shadow_update_paging_modes(struct vcpu *);
+static void cf_check shadow_update_paging_modes(struct vcpu *v);
 
 /* Set up the shadow-specific parts of a domain struct at start of day.
  * Called for every domain from arch_domain_create() */
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 225d4cff03c1..08dec9990e39 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -40,7 +40,7 @@ struct priv_op_ctxt {
 };
 
 /* I/O emulation helpers.  Use non-standard calling conventions. */
-void nocall load_guest_gprs(struct cpu_user_regs *);
+void nocall load_guest_gprs(struct cpu_user_regs *regs);
 void nocall save_guest_gprs(void);
 
 typedef void io_emul_stub_t(struct cpu_user_regs *);
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index d074a5bebecc..3f5ad01f1bdd 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -62,7 +62,7 @@ struct livepatch_fstate {
     uint8_t insn_buffer[LIVEPATCH_OPAQUE_SIZE];
 };
 
-int livepatch_op(struct xen_sysctl_livepatch_op *);
+int livepatch_op(struct xen_sysctl_livepatch_op *op);
 void check_for_livepatch_work(void);
 unsigned long livepatch_symbols_lookup_by_name(const char *symname);
 bool is_patch(const void *addr);
-- 
2.39.5


Re: [PATCH 2/5] x86: Name parameters in function declarations
Posted by Nicola Vetrini 1 day, 23 hours ago
On 2025-12-10 19:30, Andrew Cooper wrote:
> With the wider testing, some more violations have been spotted.  This
> addresses violations of Rule 8.2 (parameters must be named).
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

One nit below

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: consulting@bugseng.com <consulting@bugseng.com>
> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
>  xen/arch/x86/mm/shadow/common.c | 8 ++++----
>  xen/arch/x86/pv/emul-priv-op.c  | 2 +-
>  xen/include/xen/livepatch.h     | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/shadow/common.c 
> b/xen/arch/x86/mm/shadow/common.c
> index 423764a32653..f2aee5be46a7 100644
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -69,11 +69,11 @@ const uint8_t sh_type_to_size[] = {
> 
>  DEFINE_PER_CPU(uint32_t,trace_shadow_path_flags);
> 
> -static int cf_check sh_enable_log_dirty(struct domain *);
> -static int cf_check sh_disable_log_dirty(struct domain *);
> -static void cf_check sh_clean_dirty_bitmap(struct domain *);
> +static int cf_check sh_enable_log_dirty(struct domain *d);
> +static int cf_check sh_disable_log_dirty(struct domain *d);
> +static void cf_check sh_clean_dirty_bitmap(struct domain *d);
> 
> -static void cf_check shadow_update_paging_modes(struct vcpu *);
> +static void cf_check shadow_update_paging_modes(struct vcpu *v);
> 
>  /* Set up the shadow-specific parts of a domain struct at start of 
> day.
>   * Called for every domain from arch_domain_create() */
> diff --git a/xen/arch/x86/pv/emul-priv-op.c 
> b/xen/arch/x86/pv/emul-priv-op.c
> index 225d4cff03c1..08dec9990e39 100644
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -40,7 +40,7 @@ struct priv_op_ctxt {
>  };
> 
>  /* I/O emulation helpers.  Use non-standard calling conventions. */
> -void nocall load_guest_gprs(struct cpu_user_regs *);
> +void nocall load_guest_gprs(struct cpu_user_regs *regs);
>  void nocall save_guest_gprs(void);
> 
>  typedef void io_emul_stub_t(struct cpu_user_regs *);
> diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
> index d074a5bebecc..3f5ad01f1bdd 100644
> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -62,7 +62,7 @@ struct livepatch_fstate {
>      uint8_t insn_buffer[LIVEPATCH_OPAQUE_SIZE];
>  };
> 
> -int livepatch_op(struct xen_sysctl_livepatch_op *);
> +int livepatch_op(struct xen_sysctl_livepatch_op *op);

xen/common/livepatch.c:int livepatch_op(struct xen_sysctl_livepatch_op 
*livepatch)

Shouldn't this decl also use "*op" as well? Might not be triggered in 
this configuration due to the absence of CONFIG_LIVEPATCH I think.

>  void check_for_livepatch_work(void);
>  unsigned long livepatch_symbols_lookup_by_name(const char *symname);
>  bool is_patch(const void *addr);

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH 2/5] x86: Name parameters in function declarations
Posted by Andrew Cooper 17 hours ago
On 10/12/2025 8:15 pm, Nicola Vetrini wrote:
> On 2025-12-10 19:30, Andrew Cooper wrote:
>> With the wider testing, some more violations have been spotted.  This
>> addresses violations of Rule 8.2 (parameters must be named).
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Thanks.

>
> One nit below
>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: consulting@bugseng.com <consulting@bugseng.com>
>> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>>  xen/arch/x86/mm/shadow/common.c | 8 ++++----
>>  xen/arch/x86/pv/emul-priv-op.c  | 2 +-
>>  xen/include/xen/livepatch.h     | 2 +-
>>  3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/x86/mm/shadow/common.c
>> b/xen/arch/x86/mm/shadow/common.c
>> index 423764a32653..f2aee5be46a7 100644
>> --- a/xen/arch/x86/mm/shadow/common.c
>> +++ b/xen/arch/x86/mm/shadow/common.c
>> @@ -69,11 +69,11 @@ const uint8_t sh_type_to_size[] = {
>>
>>  DEFINE_PER_CPU(uint32_t,trace_shadow_path_flags);
>>
>> -static int cf_check sh_enable_log_dirty(struct domain *);
>> -static int cf_check sh_disable_log_dirty(struct domain *);
>> -static void cf_check sh_clean_dirty_bitmap(struct domain *);
>> +static int cf_check sh_enable_log_dirty(struct domain *d);
>> +static int cf_check sh_disable_log_dirty(struct domain *d);
>> +static void cf_check sh_clean_dirty_bitmap(struct domain *d);
>>
>> -static void cf_check shadow_update_paging_modes(struct vcpu *);
>> +static void cf_check shadow_update_paging_modes(struct vcpu *v);
>>
>>  /* Set up the shadow-specific parts of a domain struct at start of day.
>>   * Called for every domain from arch_domain_create() */
>> diff --git a/xen/arch/x86/pv/emul-priv-op.c
>> b/xen/arch/x86/pv/emul-priv-op.c
>> index 225d4cff03c1..08dec9990e39 100644
>> --- a/xen/arch/x86/pv/emul-priv-op.c
>> +++ b/xen/arch/x86/pv/emul-priv-op.c
>> @@ -40,7 +40,7 @@ struct priv_op_ctxt {
>>  };
>>
>>  /* I/O emulation helpers.  Use non-standard calling conventions. */
>> -void nocall load_guest_gprs(struct cpu_user_regs *);
>> +void nocall load_guest_gprs(struct cpu_user_regs *regs);
>>  void nocall save_guest_gprs(void);
>>
>>  typedef void io_emul_stub_t(struct cpu_user_regs *);
>> diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
>> index d074a5bebecc..3f5ad01f1bdd 100644
>> --- a/xen/include/xen/livepatch.h
>> +++ b/xen/include/xen/livepatch.h
>> @@ -62,7 +62,7 @@ struct livepatch_fstate {
>>      uint8_t insn_buffer[LIVEPATCH_OPAQUE_SIZE];
>>  };
>>
>> -int livepatch_op(struct xen_sysctl_livepatch_op *);
>> +int livepatch_op(struct xen_sysctl_livepatch_op *op);
>
> xen/common/livepatch.c:int livepatch_op(struct xen_sysctl_livepatch_op
> *livepatch)
>
> Shouldn't this decl also use "*op" as well? Might not be triggered in
> this configuration due to the absence of CONFIG_LIVEPATCH I think.

Yes, I've converted the main function to use op too.  It makes the patch
a bit larger, but it's a much better name to use in this context.

~Andrew