Re: [PATCH] KVM: arm64: Remove spurious semicolon in reg_to_encoding()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-01-05 13:54, Alexandru Elisei wrote:
Hi Marc,

On 12/31/20 3:09 PM, Marc Zyngier wrote:
Although not a problem right now, it flared up while working
on some other aspects of the code-base. Remove the useless
semicolon.

Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
---
 arch/arm64/kvm/sys_regs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index d46e7f706cb0..42ccc27fb684 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -923,7 +923,7 @@ static bool access_pmuserenr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,

 #define reg_to_encoding(x)						\
 	sys_reg((u32)(x)->Op0, (u32)(x)->Op1,				\
-		(u32)(x)->CRn, (u32)(x)->CRm, (u32)(x)->Op2);
+		(u32)(x)->CRn, (u32)(x)->CRm, (u32)(x)->Op2)

Macro's shouldn't really end with a semicolon, otherwise code might break
unexpectedly:

Reviewed-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>

I skimmed over the code and the macro is used like a function. Do you think it's
worth turning it into a function to gain type checking for the
argument (which is
expected to be sys_reg_desc) and for the return value?

Not quite.

This macro takes indifferently a sys_reg_desc or a sys_reg_params,
making it impossible to turn into a function. Is it disgusting and
makes you want to pull your eyes out? Yes.

The whole params/desc duality dates back to the early 32bit code,
and I blame Rusty Russell for it. Just saying... ;-)

That's one of things I always wanted to refactor...

Thanks,

        M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux