On Tue, Apr 29, 2025 at 12:19:49 +0200, Jiri Denemark via Devel wrote:
From: Jiri Denemark <jdenemar(a)redhat.com>
Refactor weight calculation to a separate virCPUx86WeightFeatures
function to avoid code duplication.
Signed-off-by: Jiri Denemark <jdenemar(a)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(a)redhat.com>