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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxxxx>
> Reviewed-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>

Thanks.

>
> One nit below
>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> CC: consulting@xxxxxxxxxxx <consulting@xxxxxxxxxxx>
>> CC: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
>> ---
>>  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