Your fixes are better than mine. I have tested your fixes in SPARC and
x86 machine with all the possible range of timeout values.
Thanks and Regards
-Arvind
On 04/14/10 08:08, Keith Packard wrote:
On Wed, 14 Apr 2010 07:22:24 +0530, Arvind Umrao<[email protected]> wrote:
Keith Packard wrote:
On Tue, 13 Apr 2010 16:50:30 +0530, Arvind Umrao<[email protected]> wrote:
On 04/13/10 10:57, Keith Packard wrote:
On Tue, 13 Apr 2010 10:16:20 +0530, Arvind Umrao<[email protected]> wrote:
CARD32 maxSecs = (CARD32)(~0) / (CARD32)MILLI_PER_SECOND;
+ CARD32 nowSec = GetTimeInMillis()/ (CARD32)MILLI_PER_SECOND;
- if (seconds> maxSecs)
- { /* only come here if we want to wait more than 49 days */
- pAuth->secondsRemaining = seconds - maxSecs;
- return maxSecs * MILLI_PER_SECOND;
+ CARD32 maxPossibleSec = maxSecs - nowSec;
What happens when GetTimeInMillis() returns (CARD32)(~0)?
Yes I agree you found problem in my fix. When GetTimeInMillis() returns
(CARD32)(~0) , execution should go to overflow timeout, not in regular
timeout. I have made the correction please review it and give your
comments again.
The actual requirement for the milliseconds value is that it not
fail the comparison below:
if ((int) (millis - now)<= 0)
This means keeping the milliseconds value within 31 bits of the now
value. There should be no reason to ever use GetTimeInMillis() to range
check the incoming value.
Please again, reconfirm should I remove GetTimeInMillis(). If I do not
keep the milliseconds value within 31 bits of 'now' value, Security
Authorization will get expired immediately with setting timeout to zero,
when user supply large value of timeout.
Go look at TimerSet again. For relative timers, it does the following:
timer->delta = millis;
millis += now;
if ((int) (millis - now)<= 0) {
...
All you have to do is ensure that 'millis' is less than MAXINT as
TimerSet adds 'now' and then subtracts it back off before comparing with
0.
The only change needed here was to change the ~0 to MAXINT, plus the
removal of the assert.
I think this patch will suffice (it compiles, but I haven't run it to check):
diff --git a/Xext/security.c b/Xext/security.c
index af8d205..e81e560 100644
--- a/Xext/security.c
+++ b/Xext/security.c
@@ -279,10 +279,10 @@ SecurityComputeAuthorizationTimeout(
/* maxSecs is the number of full seconds that can be expressed in
* 32 bits worth of milliseconds
*/
- CARD32 maxSecs = (CARD32)(~0) / (CARD32)MILLI_PER_SECOND;
+ CARD32 maxSecs = (CARD32)(MAXINT) / (CARD32)MILLI_PER_SECOND;
if (seconds> maxSecs)
- { /* only come here if we want to wait more than 49 days */
+ { /* only come here if we want to wait more than 24 days */
pAuth->secondsRemaining = seconds - maxSecs;
return maxSecs * MILLI_PER_SECOND;
}
@@ -320,8 +320,6 @@ SecurityAuthorizationExpired(
{
SecurityAuthorizationPtr pAuth = (SecurityAuthorizationPtr)pval;
- assert(pAuth->timer == timer);
-
if (pAuth->secondsRemaining)
{
return SecurityComputeAuthorizationTimeout(pAuth,
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
