Re: setitimer will need a range check

[John Harper <jsh unfactored org>]

Hi,

It's a long time since I've looked at this code, but it looks like  
this is what the fix_time function in that file is trying to achieve -  
i.e. ensure that 0 <= msecs < 1000. I think the change below should  
use that function to fix the bad values to setitimer, can you confirm  
that? thanks,

	John


diff -u -p -r1.15 timers.c
--- timers.c	2 Mar 2002 05:22:53 -0000	1.15
+++ timers.c	29 Dec 2006 20:39:56 -0000
@@ -252,6 +252,7 @@ to re-enable it.
     t->function = fun;
     t->secs = rep_get_long_int (secs);
     t->msecs = rep_get_long_int (msecs);
+    fix_time (&t->secs, &t->msecs);
     t->next_alloc = allocated_timers;
     allocated_timers = t;
     insert_timer (t);
@@ -290,6 +291,7 @@ duration. Otherwise, the existing values
     {
 	TIMER(timer)->secs = rep_get_long_int (secs);
 	TIMER(timer)->msecs = rep_get_long_int (msecs);
+	fix_time (&TIMER (timer)->secs, &TIMER (timer)->msecs);
     }
     insert_timer (TIMER(timer));
     return timer;



On Dec 26, 2006, at 4:35 PM, Michal Jaegermann wrote:

> The current version of 'man setitimer' for Linux says the following:
>
>        POSIX.1-2001 says that setitimer() should fail if a  tv_usec   
> value  is
>        specified  that  is  outside  of the range 0 to 999999.   
> However, Linux
>        does not give an error, but instead silently adjusts the   
> corresponding
>        seconds value for the timer.  In the future (scheduled for  
> March 2007),
>        this non-conformance will be repaired: existing applications  
> should  be
>        fixed now to ensure that they supply a properly formed  
> tv_usec value.
>
> With a library which is current enough I am seeing such entries
> in log files:
>
> setitimer: sawfish (pid = 3894) provided invalid timeval it_value:
> tv_sec = 59 tv_usec = 1000000
>
> To satisfy the above the follwing change in librep would do:
>
> --- librep/src/timers.c.itimer	2002-03-01 22:22:53.000000000 -0700
> +++ librep/src/timers.c	2006-12-26 17:00:14.000000000 -0700
> @@ -106,9 +106,12 @@ setup_next_timer (void)
>      {
>  #ifdef HAVE_SETITIMER
>  	struct itimerval it, tem;
> +	long usec;
>  	it.it_interval.tv_usec = 0;
>  	it.it_interval.tv_sec = 0;
> -	it.it_value.tv_usec = timer_chain->rel_msecs * 1000;
> +	usec = timer_chain->rel_msecs * 1000;
> +	usec = usec < 1000000 ? (usec < 0 ? 0 : usec ) : 999999;
> +	it.it_value.tv_usec = usec;
>  	it.it_value.tv_sec = timer_chain->rel_secs;
>  	setitimer (ITIMER_REAL, &it, &tem);
>  #else
>
> Strictly speaking one should likely reduce the range for tv_usec and
> adjust tv_sec value accordingly, ie. 60 and 0 in the situation  
> recorded
> in logs, but in practice the above should likely be enough.
>
>    Michal