On 3/6/20 8:11 AM, Eli Zaretskii wrote:

> (the comments in that part of the code can really use some
> enhancement; they currently seem to target only experts in both time
> handling and GMP: many macros whose names don't explain what they do,
> and plethora of calls to libgmp functions that have no comments
> whatsoever).

Although comments in that area no doubt could use improvement, I'd rather not 
see comments like this:

       /* Add TM_YEAR_BASE to mpz[0].  */
       mpz_add_ui (mpz[0], mpz[0], TM_YEAR_BASE);

as they would be the rough equivalent of:

       /* Add 1 to I.  */
       i++;

> +  (when (numberp time)
> +    (setq time (timer-relative-time nil time))
> +    (or (consp time)
> +        (setq time (time-convert time 'list))))

This would catch some problems but not all, as the real bug here is in the code 
(or (consp time) (error "Invalid time format")) which occurs a few lines later. 
As near as I can tell this later code is both wrong and unnecessary. It's wrong 
because it's no longer true that only conses are time values. It's unnecessary 
because the immediately following (timer-set-function timer function args) call 
checks the validity of TIME. On the off chance that a validity check is still 
helpful (because we don't want to create garbage?) I installed the attached 
patch. But it might be better in master to remove the "Invalid time format" 
check entirely.