Hi Phil and Manos,
On 12/18/25 10:00, Manos Pitsidianakis wrote:
On Thu, Dec 18, 2025 at 9:21 AM Philippe Mathieu-Daudé
<[email protected]> wrote:
Hi Gustavo,
On 17/12/25 22:20, Gustavo Romero wrote:
Add a sentinel to the ARMASIdx enum so it can be used when the total
number of address spaces is required.
Signed-off-by: Gustavo Romero <[email protected]>
---
target/arm/cpu.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 39f2b2e54d..00f5af0fcd 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2336,6 +2336,7 @@ typedef enum ARMASIdx {
ARMASIdx_S = 1,
ARMASIdx_TagNS = 2,
ARMASIdx_TagS = 3,
+ ARMASIdx_MAX
My two cents:
Here, "ARMASIdx_MAX" should be equal to the last variant, 3. So to get
the total count, it would be ARMASIdx_MAX + 1.
And it should be called "ARMASIdx_COUNT". Max is the last variant.
That makes total sense to me. Thanks, I'll update it in v3.
The problem with including this in the enum is this confuses static
analyzers:
warning: enumeration value 'ARMASIdx_MAX' not handled in switch [-Wswitch]
hmm I guess I missed it because my test pipeline didn't check for it?
I've used QEMU_CI=2 to test the series. But I guess one needs access
to a pipeline with Coverity enabled? How can we test for warnings like
it in the CI?
If we add a "MAX" instead of "COUNT" variant, then the value of the
MAX variant would be handled in switch cases. The following
definition does not emit a warning with -Wswitch:
typedef enum ARMASIdx {
ARMASIdx_NS = 0,
ARMASIdx_S = 1,
ARMASIdx_TagNS = 2,
ARMASIdx_TagS = 3,
ARMASIdx_MAX = ARMASIdx_TagS,
#define ARMASIdx_COUNT (ARMASIdx_MAX + 1)
} ARMASIdx;
int main() {
ARMASIdx t = ARMASIdx_S;
switch (t) {
case ARMASIdx_NS:
break;
case ARMASIdx_S:
break;
case ARMASIdx_TagNS:
break;
case ARMASIdx_TagS:
break;
}
printf("Last = %d Count = %d\n", ARMASIdx_MAX, ARMASIdx_COUNT);
}
Outputs:
"Last = 3 Count = 4"
Nice :)
Cheers,
Gustavo
To avoid that we /define/ it manually instead:
#define ARMASIdx_MAX 4
} ARMASIdx;
Usually the definition is within the enum declaration, and we name
it ${enum}_COUNT:
typedef enum ARMASIdx {
ARMASIdx_NS = 0,
ARMASIdx_S = 1,
ARMASIdx_TagNS = 2,
ARMASIdx_TagS = 3,
#define ARMASIdx_COUNT 4
} ARMASIdx;
Unfortunately this didn't work well with QAPI, so we could never enable
-Wswitch globally:
https://lore.kernel.org/qemu-devel/[email protected]/
Today I'm not sure what is the best style anymore, so just take
my comments are historical 2 cents.
Regards,
Phil.
