On Tue, Apr 29, 2025 at 12:19:49 +0200, Jiri Denemark via Devel wrote:
From: Jiri Denemark <jdenemar@redhat.com>
Refactor weight calculation to a separate virCPUx86WeightFeatures function to avoid code duplication.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 83 ++++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 37 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 32aa01bc14..b0fe2bed4c 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2090,63 +2090,72 @@ virCPUx86Compare(virCPUDef *host, /* Base penalty for disabled features. */ #define BASE_PENALTY 2
+struct virCPUx86Weight { + size_t total; + size_t enabled; + size_t disabled; +}; + +static void +virCPUx86WeightFeatures(const virCPUDef *cpu, + struct virCPUx86Weight *weight) +{ + int penalty = BASE_PENALTY; + size_t i; + + weight->enabled = cpu->nfeatures; + weight->disabled = 0; + + if (cpu->type == VIR_CPU_TYPE_HOST) { + weight->total = cpu->nfeatures; + return; + } + + for (i = 0; i < weight->enabled; i++) { + if (cpu->features[i].policy == VIR_CPU_FEATURE_DISABLE) { + weight->enabled--;
I know that this is just moving code but the fact that you modify the variable used to limit the iteration ought to be explained in a comment because it's really non-obvious to a reader what's happening here.
+ weight->disabled += penalty; + penalty++; + } + } + + weight->total = weight->enabled + weight->disabled; +} + +
The new version is much more readable! Reviewed-by: Peter Krempa <pkrempa@redhat.com>