On Sun, Apr 12, 2026 at 11:30 AM Andres Freund <[email protected]> wrote:
>
> On 2026-04-12 14:20:31 -0400, Tom Lane wrote:
> > 166 uint64 loop_count;
> > 167
> > 168 loop_count = test_timing(test_duration,
> > TIMING_CLOCK_SOURCE_SYSTEM, false);
> > >>> CID 1691465: Incorrect expression (DIVIDE_BY_ZERO)
> > >>> In function call "output", division by expression "loop_count"
> > >>> which may be zero has undefined behavior.
> > 169 output(loop_count);
> >
> > AFAICS it's correct to complain. test_timing() visibly can return zero,
> > but of the three places where test_timing() is followed by output()
> > only one has a defense against that.
>
> I think it should be unreachable as-is (but we should fix it anyway): If the
> system clock source doesn't work, we have much bigger issues. If rdtscp works,
> rdtsc should better work as well...
Yeah, agreed, in practice this won't be reached, but good to fix.
> Maybe it's enough to add an Assert() to clarify this? But I guess just
> printing a message in that unreachable case would also work.
I think either is fine. If we did it with a message, how about this at
the beginning of the output function?
if (loop_count == 0)
{
printf(_("WARNING: No timing measurements collected. Report this as
a bug to <%s>.\n"), PACKAGE_BUGREPORT);
return;
}
That'd align with the proposed patch to warn about calibrated TSC
frequency diverging more than 10%.
Thanks,
Lukas
--
Lukas Fittl
