Commit 143e7eea authored by Sean Christopherson's avatar Sean Christopherson Committed by Paolo Bonzini

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: default avatarSean Christopherson <seanjc@google.com>
Reviewed-by: default avatarDavid Matlack <dmatlack@google.com>
Signed-off-by: default avatarBen Gardon <bgardon@google.com>
Message-Id: <20220613212523.3436117-5-bgardon@google.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 4d0a0594
...@@ -40,27 +40,31 @@ static void stats_test(int stats_fd) ...@@ -40,27 +40,31 @@ static void stats_test(int stats_fd)
/* Read kvm stats id string */ /* Read kvm stats id string */
id = malloc(header.name_size); id = malloc(header.name_size);
TEST_ASSERT(id, "Allocate memory for id string"); TEST_ASSERT(id, "Allocate memory for id string");
ret = read(stats_fd, id, header.name_size); ret = read(stats_fd, id, header.name_size);
TEST_ASSERT(ret == header.name_size, "Read id string"); TEST_ASSERT(ret == header.name_size, "Read id string");
/* Check id string, that should start with "kvm" */ /* Check id string, that should start with "kvm" */
TEST_ASSERT(!strncmp(id, "kvm", 3) && strlen(id) < header.name_size, 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 */ /* Sanity check for other fields in header */
if (header.num_desc == 0) { if (header.num_desc == 0) {
printf("No KVM stats defined!"); printf("No KVM stats defined!");
return; return;
} }
/* Check overlap */ /*
TEST_ASSERT(header.desc_offset > 0 && header.data_offset > 0 * The descriptor and data offsets must be valid, they must not overlap
&& header.desc_offset >= sizeof(header) * the header, and the descriptor and data blocks must not overlap each
&& header.data_offset >= sizeof(header), * other. Note, the data block is rechecked after its size is known.
"Invalid offset fields in header"); */
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 || TEST_ASSERT(header.desc_offset > header.data_offset ||
(header.desc_offset + size_desc * header.num_desc <= (header.desc_offset + size_desc * header.num_desc <= header.data_offset),
header.data_offset), "Descriptor block is overlapped with data block");
"Descriptor block is overlapped with data block");
/* Read kvm stats descriptors */ /* Read kvm stats descriptors */
stats_desc = read_stats_descriptors(stats_fd, &header); stats_desc = read_stats_descriptors(stats_fd, &header);
...@@ -68,14 +72,17 @@ static void stats_test(int stats_fd) ...@@ -68,14 +72,17 @@ static void stats_test(int stats_fd)
/* Sanity check for fields in descriptors */ /* Sanity check for fields in descriptors */
for (i = 0; i < header.num_desc; ++i) { for (i = 0; i < header.num_desc; ++i) {
pdesc = get_stats_descriptor(stats_desc, i, &header); pdesc = get_stats_descriptor(stats_desc, i, &header);
/* Check type,unit,base boundaries */ /* Check type,unit,base boundaries */
TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK) TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK) <= KVM_STATS_TYPE_MAX,
<= KVM_STATS_TYPE_MAX, "Unknown KVM stats type"); "Unknown KVM stats type");
TEST_ASSERT((pdesc->flags & KVM_STATS_UNIT_MASK) TEST_ASSERT((pdesc->flags & KVM_STATS_UNIT_MASK) <= KVM_STATS_UNIT_MAX,
<= KVM_STATS_UNIT_MAX, "Unknown KVM stats unit"); "Unknown KVM stats unit");
TEST_ASSERT((pdesc->flags & KVM_STATS_BASE_MASK) TEST_ASSERT((pdesc->flags & KVM_STATS_BASE_MASK) <= KVM_STATS_BASE_MAX,
<= KVM_STATS_BASE_MAX, "Unknown KVM stats base"); "Unknown KVM stats base");
/* Check exponent for stats unit
/*
* Check exponent for stats unit
* Exponent for counter should be greater than or equal to 0 * 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 bytes should be greater than or equal to 0
* Exponent for unit seconds should be less 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) ...@@ -86,47 +93,51 @@ static void stats_test(int stats_fd)
case KVM_STATS_UNIT_NONE: case KVM_STATS_UNIT_NONE:
case KVM_STATS_UNIT_BYTES: case KVM_STATS_UNIT_BYTES:
case KVM_STATS_UNIT_CYCLES: case KVM_STATS_UNIT_CYCLES:
TEST_ASSERT(pdesc->exponent >= 0, TEST_ASSERT(pdesc->exponent >= 0, "Unsupported KVM stats unit");
"Unsupported KVM stats unit");
break; break;
case KVM_STATS_UNIT_SECONDS: case KVM_STATS_UNIT_SECONDS:
TEST_ASSERT(pdesc->exponent <= 0, TEST_ASSERT(pdesc->exponent <= 0, "Unsupported KVM stats unit");
"Unsupported KVM stats unit");
break; break;
} }
/* Check name string */ /* Check name string */
TEST_ASSERT(strlen(pdesc->name) < header.name_size, 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 */ /* Check size field, which should not be zero */
TEST_ASSERT(pdesc->size, "KVM descriptor(%s) with size of 0", TEST_ASSERT(pdesc->size,
pdesc->name); "KVM descriptor(%s) with size of 0", pdesc->name);
/* Check bucket_size field */ /* Check bucket_size field */
switch (pdesc->flags & KVM_STATS_TYPE_MASK) { switch (pdesc->flags & KVM_STATS_TYPE_MASK) {
case KVM_STATS_TYPE_LINEAR_HIST: case KVM_STATS_TYPE_LINEAR_HIST:
TEST_ASSERT(pdesc->bucket_size, TEST_ASSERT(pdesc->bucket_size,
"Bucket size of Linear Histogram stats (%s) is zero", "Bucket size of Linear Histogram stats (%s) is zero",
pdesc->name); pdesc->name);
break; break;
default: default:
TEST_ASSERT(!pdesc->bucket_size, TEST_ASSERT(!pdesc->bucket_size,
"Bucket size of stats (%s) is not zero", "Bucket size of stats (%s) is not zero",
pdesc->name); pdesc->name);
} }
size_data += pdesc->size * sizeof(*stats_data); 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, * Now that the size of the data block is known, verify the data block
"Data block is overlapped with Descriptor 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 */ /* Check validity of all stats data size */
TEST_ASSERT(size_data >= header.num_desc * sizeof(*stats_data), TEST_ASSERT(size_data >= header.num_desc * sizeof(*stats_data),
"Data size is not correct"); "Data size is not correct");
/* Check stats offset */ /* Check stats offset */
for (i = 0; i < header.num_desc; ++i) { for (i = 0; i < header.num_desc; ++i) {
pdesc = get_stats_descriptor(stats_desc, i, &header); pdesc = get_stats_descriptor(stats_desc, i, &header);
TEST_ASSERT(pdesc->offset < size_data, TEST_ASSERT(pdesc->offset < size_data,
"Invalid offset (%u) for stats: %s", "Invalid offset (%u) for stats: %s",
pdesc->offset, pdesc->name); pdesc->offset, pdesc->name);
} }
/* Allocate memory for stats data */ /* Allocate memory for stats data */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment