Skip to content

Commit

Permalink
KVM: selftests: Clean up coding style in binary stats test
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Sean Christopherson authored and Paolo Bonzini committed Jun 24, 2022
1 parent 4d0a059 commit 143e7ee
Showing 1 changed file with 45 additions and 34 deletions.
79 changes: 45 additions & 34 deletions tools/testing/selftests/kvm/kvm_binary_stats_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,42 +40,49 @@ 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);

/* 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
Expand All @@ -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 */
Expand Down

0 comments on commit 143e7ee

Please sign in to comment.