On Tue, 30 Apr 2024 at 18:31, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> On 4/30/24 07:00, Peter Maydell wrote:
> > + if (uadd64_overflow(timeout, offset, &nexttick)) {
> > + nexttick = UINT64_MAX;
> > + }
> > + if (nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
> > + /*
> > + * If the timeout is too long for the signed 64-bit range
> > + * of a QEMUTimer, let it expire early.
> > + */
> > + timer_mod_ns(cpu->wfxt_timer, INT64_MAX);
> > + } else {
> > + timer_mod(cpu->wfxt_timer, nexttick);
> > + }
>
> The use of both UINT64_MAX and INT64_MAX is confusing. Perhaps
>
> if (uadd64_overflow(timeout, offset, &nexttick) ||
> nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
> nexttick = INT64_MAX;
> }
> timer_mod(cpu->wfxt_timer, nexttick);
I'm following here the pattern of the logic in gt_recalc_timer()
(which could admittedly also be considered confusing...).
Also note that timer_mod_ns() and timer_mod() aren't the
same thing. The latter calls timer_mod_ns() on its argument
multiplied by ts->scale, so if you pass it INT64_MAX
the multiply is liable to overflow.
thanks
-- PMM
