On Thu, Jan 22, 2026 at 10:28:51AM +0100, Jan Beulich wrote:
> On 22.01.2026 10:18, Roger Pau Monné wrote:
> > On Mon, Nov 17, 2025 at 03:39:30PM +0100, Jan Beulich wrote:
> >> When this was added, the log message was updated correctly, but the zero
> >> case was needlessly checked separately: hpet_broadcast_enter() had a zero
> >> check added at the same time, while handle_hpet_broadcast() can't possibly
> >> pass 0 here anyway.
> >>
> >> Fixes: 7145897cfb81 ("cpuidle: Fix for timer_deadline==0 case")
> >> Signed-off-by: Jan Beulich <[email protected]>
> >
> > Acked-by: Roger Pau Monné <[email protected]>
>
> Thanks.
>
> > Similar to the previous commit, I wonder whether it would make sense
> > to add an ASSERT_UNREACHABLE() if that error path is not reachable
> > given the logic in the callers.
>
> That would mean
>
> if ( unlikely(expire < 0) )
> {
> printk(KERN_DEBUG "reprogram: expire <= 0\n");
> return -ETIME;
> }
>
> if ( unlikely(expire == 0) )
> {
> ASSERT_UNREACHABLE();
> return -ETIME;
> }
>
> which I fear I don't like (for going too far). Even
>
> if ( unlikely(expire <= 0) )
> {
> printk(KERN_DEBUG "reprogram: expire <= 0\n");
> ASSERT(expire);
> return -ETIME;
> }
>
> I'd be uncertain about, as that needlessly gives 0 a meaning that isn't
> required anymore in this function.
Hm, OK, I was under the impression that both < 0 and 0 should never be
passed by the callers. If expire == 0 is a possible input then I
don't think the ASSERT() is that helpful.
Thanks, Roger.