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.
In addition, I'd suggest that the assert in SecurityAuthorizationExpired
be removed; that's a bogus test with pAuth->timer is NULL, as it will be
when TimerSet is first invoked.
Yes I do agree with you. Assert in SecurityAuthorizationExpired should be removed and it is bogus. I will recreate patch and sent it for review.
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to