Peter Maydell <peter.mayd...@linaro.org> writes: > 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 for the report. Peter Xu is not available at the moment, I'll work on this.