KVM: selftests: Clean up coding style in binary stats test

Fix a variety of code style violations and/or inconsistencies in the
binary stats test.  The 80 char limit is a soft limit and can and should
be ignored/violated if doing so improves the overall code readability.

Specifically, provide consistent indentation and don't split expressions
at arbitrary points just to honor the 80 char limit.

Opportunistically expand/add comments to call out the more subtle aspects
of the code.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Matlack <dmatlack@google.com>
Signed-off-by: Ben Gardon <bgardon@google.com>
Message-Id: <20220613212523.3436117-5-bgardon@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/tools/testing/selftests/kvm/kvm_binary_stats_test.c b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
index 66cf6cd..a904873 100644 (file)
--- a/tools/testing/selftests/kvm/kvm_binary_stats_test.c
+++ b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
@@ -40,27 +40,31 @@ static void stats_test(int stats_fd)
        /* Read kvm stats id string */
        id = malloc(header.name_size);
        TEST_ASSERT(id, "Allocate memory for id string");
+
        ret = read(stats_fd, id, header.name_size);
        TEST_ASSERT(ret == header.name_size, "Read id string");
 
        /* Check id string, that should start with "kvm" */
        TEST_ASSERT(!strncmp(id, "kvm", 3) && strlen(id) < header.name_size,
-                               "Invalid KVM stats type, id: %s", id);
+                   "Invalid KVM stats type, id: %s", id);
 
        /* Sanity check for other fields in header */
        if (header.num_desc == 0) {
                printf("No KVM stats defined!");
                return;
        }
-       /* Check overlap */
-       TEST_ASSERT(header.desc_offset > 0 && header.data_offset > 0
-                       && header.desc_offset >= sizeof(header)
-                       && header.data_offset >= sizeof(header),
-                       "Invalid offset fields in header");
+       /*
+        * The descriptor and data offsets must be valid, they must not overlap
+        * the header, and the descriptor and data blocks must not overlap each
+        * other.  Note, the data block is rechecked after its size is known.
+        */
+       TEST_ASSERT(header.desc_offset && header.desc_offset >= sizeof(header) &&
+                   header.data_offset && header.data_offset >= sizeof(header),
+                   "Invalid offset fields in header");
+
        TEST_ASSERT(header.desc_offset > header.data_offset ||
-                       (header.desc_offset + size_desc * header.num_desc <=
-                                                       header.data_offset),
-                       "Descriptor block is overlapped with data block");
+                   (header.desc_offset + size_desc * header.num_desc <= header.data_offset),
+                   "Descriptor block is overlapped with data block");
 
        /* Read kvm stats descriptors */
        stats_desc = read_stats_descriptors(stats_fd, &header);
@@ -68,14 +72,17 @@ static void stats_test(int stats_fd)
        /* Sanity check for fields in descriptors */
        for (i = 0; i < header.num_desc; ++i) {
                pdesc = get_stats_descriptor(stats_desc, i, &header);
+
                /* Check type,unit,base boundaries */
-               TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK)
-                               <= KVM_STATS_TYPE_MAX, "Unknown KVM stats type");
-               TEST_ASSERT((pdesc->flags & KVM_STATS_UNIT_MASK)
-                               <= KVM_STATS_UNIT_MAX, "Unknown KVM stats unit");
-               TEST_ASSERT((pdesc->flags & KVM_STATS_BASE_MASK)
-                               <= KVM_STATS_BASE_MAX, "Unknown KVM stats base");
-               /* Check exponent for stats unit
+               TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK) <= KVM_STATS_TYPE_MAX,
+                           "Unknown KVM stats type");
+               TEST_ASSERT((pdesc->flags & KVM_STATS_UNIT_MASK) <= KVM_STATS_UNIT_MAX,
+                           "Unknown KVM stats unit");
+               TEST_ASSERT((pdesc->flags & KVM_STATS_BASE_MASK) <= KVM_STATS_BASE_MAX,
+                           "Unknown KVM stats base");
+
+               /*
+                * Check exponent for stats unit
                 * Exponent for counter should be greater than or equal to 0
                 * Exponent for unit bytes should be greater than or equal to 0
                 * Exponent for unit seconds should be less than or equal to 0
@@ -86,47 +93,51 @@ static void stats_test(int stats_fd)
                case KVM_STATS_UNIT_NONE:
                case KVM_STATS_UNIT_BYTES:
                case KVM_STATS_UNIT_CYCLES:
-                       TEST_ASSERT(pdesc->exponent >= 0,
-                                       "Unsupported KVM stats unit");
+                       TEST_ASSERT(pdesc->exponent >= 0, "Unsupported KVM stats unit");
                        break;
                case KVM_STATS_UNIT_SECONDS:
-                       TEST_ASSERT(pdesc->exponent <= 0,
-                                       "Unsupported KVM stats unit");
+                       TEST_ASSERT(pdesc->exponent <= 0, "Unsupported KVM stats unit");
                        break;
                }
                /* Check name string */
                TEST_ASSERT(strlen(pdesc->name) < header.name_size,
-                               "KVM stats name(%s) too long", pdesc->name);
+                           "KVM stats name(%s) too long", pdesc->name);
                /* Check size field, which should not be zero */
-               TEST_ASSERT(pdesc->size, "KVM descriptor(%s) with size of 0",
-                               pdesc->name);
+               TEST_ASSERT(pdesc->size,
+                           "KVM descriptor(%s) with size of 0", pdesc->name);
                /* Check bucket_size field */
                switch (pdesc->flags & KVM_STATS_TYPE_MASK) {
                case KVM_STATS_TYPE_LINEAR_HIST:
                        TEST_ASSERT(pdesc->bucket_size,
-                           "Bucket size of Linear Histogram stats (%s) is zero",
-                           pdesc->name);
+                                   "Bucket size of Linear Histogram stats (%s) is zero",
+                                   pdesc->name);
                        break;
                default:
                        TEST_ASSERT(!pdesc->bucket_size,
-                           "Bucket size of stats (%s) is not zero",
-                           pdesc->name);
+                                   "Bucket size of stats (%s) is not zero",
+                                   pdesc->name);
                }
                size_data += pdesc->size * sizeof(*stats_data);
        }
-       /* Check overlap */
-       TEST_ASSERT(header.data_offset >= header.desc_offset
-               || header.data_offset + size_data <= header.desc_offset,
-               "Data block is overlapped with Descriptor block");
+
+       /*
+        * Now that the size of the data block is known, verify the data block
+        * doesn't overlap the descriptor block.
+        */
+       TEST_ASSERT(header.data_offset >= header.desc_offset ||
+                   header.data_offset + size_data <= header.desc_offset,
+                   "Data block is overlapped with Descriptor block");
+
        /* Check validity of all stats data size */
        TEST_ASSERT(size_data >= header.num_desc * sizeof(*stats_data),
-                       "Data size is not correct");
+                   "Data size is not correct");
+
        /* Check stats offset */
        for (i = 0; i < header.num_desc; ++i) {
                pdesc = get_stats_descriptor(stats_desc, i, &header);
                TEST_ASSERT(pdesc->offset < size_data,
-                       "Invalid offset (%u) for stats: %s",
-                       pdesc->offset, pdesc->name);
+                           "Invalid offset (%u) for stats: %s",
+                           pdesc->offset, pdesc->name);
        }
 
        /* Allocate memory for stats data */