On Fri, 11 Jul 2025 at 15:20, Fabiano Rosas <faro...@suse.de> wrote: > > From: Peter Xu <pet...@redhat.com> > > Add the latency distribution too for blocktime, using order-of-two buckets. > It accounts for all the faults, from either vCPU or non-vCPU threads. With > prior rework, it's very easy to achieve by adding an array to account for > faults in each buckets. > > Sample output for HMP (while for QMP it's simply an array): > > Postcopy Latency Distribution:
Hi; Coverity points out that there is a bug here (CID 1612248): > +static const gchar *format_time_str(uint64_t us) > +{ > + const char *units[] = {"us", "ms", "sec"}; > + int index = 0; > + > + while (us > 1000) { > + us /= 1000; > + if (++index >= (sizeof(units) - 1)) { sizeof(units) is the size in bytes, which in this case is 24, as it is an array of three 8-byte pointers. So it's not the right thing to use in bounds checking the array index. You probably wanted ARRAY_SIZE(units). Also, the ++index inside the comparison here seems unnecessarily confusing. I would suggest something like while (us > 1000 && index + 1 < ARRAY_SIZE(units)) { us /= 1000; index++; } which puts into the while condition the two conditions under which we are OK to shift down a unit, and keeps it clear that we maintain the invariant of "when we shift down a unit we also divide the value by 1000". > + break; > + } > + } > + > + return g_strdup_printf("%"PRIu64" %s", us, units[index]); Otherwise this units[index] access could be off the end of the array. > +} thanks -- PMM