[PATCH] KVM: arm64: vgic: add default case to switch statement

Osama Abdelkader posted 1 patch 2 days, 3 hours ago
arch/arm64/kvm/vgic/vgic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] KVM: arm64: vgic: add default case to switch statement
Posted by Osama Abdelkader 2 days, 3 hours ago
The switch statement in vgic_validate_injection() handles all enum
values for irq->config, but lacked a default case. Add one to match
the pattern used in other switch statements in the same file and make
the defensive return explicit.

Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 arch/arm64/kvm/vgic/vgic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 430aa98888fd..a035ce6231e2 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -369,9 +369,9 @@ static bool vgic_validate_injection(struct vgic_irq *irq, bool level, void *owne
 		return irq->line_level != level;
 	case VGIC_CONFIG_EDGE:
 		return level;
+	default:
+		return false;
 	}
-
-	return false;
 }
 
 static bool vgic_model_needs_bcst_kick(struct kvm *kvm)
-- 
2.43.0
Re: [PATCH] KVM: arm64: vgic: add default case to switch statement
Posted by Marc Zyngier 1 day, 16 hours ago
On Thu, 11 Dec 2025 22:40:28 +0000,
Osama Abdelkader <osama.abdelkader@gmail.com> wrote:
> 
> The switch statement in vgic_validate_injection() handles all enum
> values for irq->config, but lacked a default case. Add one to match
> the pattern used in other switch statements in the same file and make

News flash: there is no other switch statement in this file. This is
the only one.

> the defensive return explicit.

I'd suggest you read the code more carefully, and consider the
following observations:

enum vgic_irq_config {
	VGIC_CONFIG_EDGE = 0,
	VGIC_CONFIG_LEVEL
};

struct vgic_irq {
	[...]
	enum vgic_irq_config config:1;	/* Level or edge */
	[...]
};

With these two definitions in mind, let's revisit the function you are
patching:

static bool vgic_validate_injection(struct vgic_irq *irq, bool level, void *owner)
{
	[...]

	switch (irq->config) {
	case VGIC_CONFIG_LEVEL:
		return irq->line_level != level;
	case VGIC_CONFIG_EDGE:
		return level;
	}

	return false;
}

Please explain how it is possible that the switch does not cover
*exhaustively* all the possible values that are constraint to exactly
One. Single. Bit? How adding a default case makes things better? The
compiler already knows that we are covering all the possible values of
the enum (yes, even a C compiler can achieve that), so it can *prove*
that there is no need for a default.

If anything, I'd have expected a patch dropping the last return, as it
is remarkably pointless. But just moving it? What does it change?

I'd suggest you think a bit more before posting random patches and
wasting people's time. I spent a good 15 minutes writing this, which
you could have used yourself to realise this change is pointless.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.