Message-ID: <000501bf1fbf$274dfbe0$0201a8c0@center1.mivlgu.ru>
From: "Sergey Vlasov" <vsu AT au DOT ru>
To: "Eli Zaretskii" <eliz AT is DOT elta DOT co DOT il>
Cc: <djgpp-workers AT delorie DOT com>
Subject: Re: [vsu AT au DOT ru: bugs in itimer.c]
Date: Tue, 26 Oct 1999 17:14:28 +0300
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
X-Priority: 3
X-MSMail-Priority: Normal
X-Mailer: Microsoft Outlook Express 4.72.3110.5
X-MimeOLE: Produced By Microsoft MimeOLE V4.72.3110.3
Reply-To: djgpp-workers AT delorie DOT com

> I tend to include this change in the current CVS sources, but I'd like
> to give it some more testing.  If you can download the latest
> djtst203.zip (from the alphas directory on one of the usual DJGPP
> sites), run the test programs in the tests/libc/posix/signal and see
> if they still behave the same (or better) with the new version, that
> would be swell.  If not, I'll do it when I have time.

I don't know when I will have the possibility to download djtst203.zip. For
now, I tried to run only the test from djtst202.zip, and it runs fine.

>> Maybe setitimer() should be changed to use the more precise version also,
>> so that getitimer() and setitimer() will have identical behavior?

> Yes.  Please change setitimer also, to use the same conversion.

Here is the corrected diff to itimer.c. This conversion can cause overflow
with tv_usec > LONG_MAX*1000000/UCLOCKS_PER_SEC, but such large values are
out of range anyway.

I also noticed that the conversion of the tv_sec part in setitimer() was
also wrong: the value was calculated as 32-bit, limiting the maximum delay
to about an hour. Adding cast to uclock_t fixes this.

--- Sergey Vlasov <vsu AT au DOT ru>


=========================== itimer.diff ===========================
*** itimer0.c   Fri Oct 22 15:51:28 1999
--- itimer.c    Tue Oct 26 16:25:30 1999
***************
*** 34,39 ****
--- 34,56 ----

  static uclock_t u_now;

+ /* Multiply a signed 32-bit integer (val) by a signed 32-bit integer (m)
+    and divide the 64-bit intermediate result by a signed 32-bit integer (d).
+    Returns 32-bit signed integer result of the division.
+
+    In other words, calculate (long)(((long long)val * m)/d), but
+    in a more efficient way, using 32*32->64 multiplication and
+    64/32->32 division commands of Intel processors. */
+ static inline long muldiv(long val, long m, long d)
+ {
+   long rv;
+   asm( "imull %2\n\t"
+        "idivl %3"
+        : "=a" (rv) : "a" (val), "r" (m), "r" (d) : "dx" );
+   return rv;
+ }
+
+
  int
  getitimer(int which, struct itimerval *value)
  {
***************
*** 61,70 ****
      errno = EINVAL;
      return -1;
    }
!   value->it_value.tv_sec    = expire / UCLOCKS_PER_SEC;
!   value->it_value.tv_usec   = (expire % UCLOCKS_PER_SEC)*3433/4096;
!   value->it_interval.tv_sec = reload / UCLOCKS_PER_SEC;
!   value->it_interval.tv_usec= (reload % UCLOCKS_PER_SEC)*3433/4096;
    return 0;
  }

--- 78,89 ----
      errno = EINVAL;
      return -1;
    }
!   value->it_value.tv_sec     = expire / UCLOCKS_PER_SEC;
!   value->it_value.tv_usec    = muldiv(expire % UCLOCKS_PER_SEC,
!                                       1000000, UCLOCKS_PER_SEC);
!   value->it_interval.tv_sec  = reload / UCLOCKS_PER_SEC;
!   value->it_interval.tv_usec = muldiv(reload % UCLOCKS_PER_SEC,
!                                       1000000, UCLOCKS_PER_SEC);
    return 0;
  }

***************
*** 223,229 ****
         so don't return just yet.  */
    }

!   *t_rel = value->it_interval.tv_sec * UCLOCKS_PER_SEC;

    /* Posix systems expect timer values smaller than the resolution of
       the system clock be rounded up to the clock resolution.  */
--- 242,248 ----
         so don't return just yet.  */
    }

!   *t_rel = (uclock_t)value->it_interval.tv_sec * UCLOCKS_PER_SEC;

    /* Posix systems expect timer values smaller than the resolution of
       the system clock be rounded up to the clock resolution.  */
***************
*** 234,257 ****
    if (value->it_interval.tv_sec == 0 && usecs < usecs_min)
      usecs = usecs_min;

!   /* Rounding errors ?? First multiply and then divide gives an
!      overflow if the USEC member is larger than 524288. */
!   if (usecs < 524200)
!     *t_rel += (usecs * 4096) / 3433;
!   else
!     *t_rel += (usecs * 2048) / 1716;

    if ((value->it_value.tv_sec|value->it_value.tv_usec) == 0)
      return 0;

!   *t_exp = value->it_value.tv_sec * UCLOCKS_PER_SEC;
    usecs = value->it_value.tv_usec;
    if (value->it_value.tv_sec == 0 && usecs < usecs_min)
      usecs = usecs_min;
!   if (usecs < 524200)
!     *t_exp += (usecs * 4096) / 3433;
!   else
!     *t_exp += (usecs * 2048) / 1716;

    /* u_now is returned zero first time uclock() is called.  That first
       call could be the one we issued above, or it could be two days
--- 253,270 ----
    if (value->it_interval.tv_sec == 0 && usecs < usecs_min)
      usecs = usecs_min;

!   /* No more rounding errors here. And no overflows (when `usecs' is not
!      absurdly large), since the intermediate result is now 64 bits wide. */
!   *t_rel += muldiv(usecs, UCLOCKS_PER_SEC, 1000000);

    if ((value->it_value.tv_sec|value->it_value.tv_usec) == 0)
      return 0;

!   *t_exp = (uclock_t)value->it_value.tv_sec * UCLOCKS_PER_SEC;
    usecs = value->it_value.tv_usec;
    if (value->it_value.tv_sec == 0 && usecs < usecs_min)
      usecs = usecs_min;
!   *t_exp += muldiv(usecs, UCLOCKS_PER_SEC, 1000000);

    /* u_now is returned zero first time uclock() is called.  That first
       call could be the one we issued above, or it could be two days