[PATCH] xserver/Xext/sync.c: Fix wrong bracket values when startOver = FALSE (Bug 27023)
David James
davidjames at google.com
Mon May 10 14:12:22 PDT 2010
On Mon, May 10, 2010 at 1:52 PM, Keith Packard <keithp at keithp.com> wrote:
> On Fri, 12 Mar 2010 10:45:22 -0800, David James <davidjames at google.com> wrote:
>
>> Currently, the SyncComputeBracketValues function in
>> xserver/Xext/sync.c calculates bracket values incorrectly when
>> startOver = FALSE. I have attached a patch to fix this.
>
> I'll need a Signed-off-by: line to merge this patch into the server.
Sure thing. I've updated my patch to apply cleanly, and added a
Signed-off-by line and the Reviewed-by line.
Attached as 0001-Fix-wrong-bracket-values-when-startOver-FALSE-patch.txt .
Cheers,
David
-------------- next part --------------
From e622b07d76774ed215c80a4e01b7727e51743ebc Mon Sep 17 00:00:00 2001
From: David James <davidjames at google.com>
Date: Mon, 10 May 2010 14:00:49 -0700
Subject: [PATCH] Fix wrong bracket values when startOver = FALSE.
Currently, SyncComputeBracketValues reuses old values of bracket_greater
and bracket_less when startOver = FALSE. This can result in incorrect bracket
values. To fix this issue, the startOver parameter is removed, and we do not
reuse old values of bracket_greater and bracket_less.
Signed-off-by: David James <davidjames at google.com>
Reviewed-by: Adam Jackson <ajax at redhat.com>
X.Org Bug 27023 <http://bugs.freedesktop.org/show_bug.cgi?id=27023>
---
Xext/sync.c | 19 ++++++++-----------
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/Xext/sync.c b/Xext/sync.c
index 990cb67..e865e52 100644
--- a/Xext/sync.c
+++ b/Xext/sync.c
@@ -94,7 +94,7 @@ static SyncCounter **SysCounterList = NULL;
#define XSyncCAAllTrigger \
(XSyncCACounter | XSyncCAValueType | XSyncCAValue | XSyncCATestType)
-static void SyncComputeBracketValues(SyncCounter *, Bool);
+static void SyncComputeBracketValues(SyncCounter *);
static void SyncInitServerTime(void);
@@ -167,7 +167,7 @@ SyncDeleteTriggerFromCounter(SyncTrigger *pTrigger)
}
if (IsSystemCounter(pTrigger->pCounter))
- SyncComputeBracketValues(pTrigger->pCounter, /*startOver*/ TRUE);
+ SyncComputeBracketValues(pTrigger->pCounter);
}
@@ -194,7 +194,7 @@ SyncAddTriggerToCounter(SyncTrigger *pTrigger)
pTrigger->pCounter->pTriglist = pCur;
if (IsSystemCounter(pTrigger->pCounter))
- SyncComputeBracketValues(pTrigger->pCounter, /*startOver*/ TRUE);
+ SyncComputeBracketValues(pTrigger->pCounter);
return Success;
}
@@ -351,7 +351,7 @@ SyncInitTrigger(ClientPtr client, SyncTrigger *pTrigger, XSyncCounter counter,
}
else if (IsSystemCounter(pCounter))
{
- SyncComputeBracketValues(pCounter, /*startOver*/ TRUE);
+ SyncComputeBracketValues(pCounter);
}
return Success;
@@ -646,7 +646,7 @@ SyncChangeCounter(SyncCounter *pCounter, CARD64 newval)
if (IsSystemCounter(pCounter))
{
- SyncComputeBracketValues(pCounter, /* startOver */ FALSE);
+ SyncComputeBracketValues(pCounter);
}
}
@@ -913,7 +913,7 @@ SyncDestroySystemCounter(pointer pSysCounter)
}
static void
-SyncComputeBracketValues(SyncCounter *pCounter, Bool startOver)
+SyncComputeBracketValues(SyncCounter *pCounter)
{
SyncTriggerList *pCur;
SyncTrigger *pTrigger;
@@ -930,11 +930,8 @@ SyncComputeBracketValues(SyncCounter *pCounter, Bool startOver)
if (ct == XSyncCounterNeverChanges)
return;
- if (startOver)
- {
- XSyncMaxValue(&psci->bracket_greater);
- XSyncMinValue(&psci->bracket_less);
- }
+ XSyncMaxValue(&psci->bracket_greater);
+ XSyncMinValue(&psci->bracket_less);
for (pCur = pCounter->pTriglist; pCur; pCur = pCur->next)
{
--
1.7.0.1
Reproduction recipe:
1. Setup a positive transition alarm for when idle time exceeds 2000ms
2. Setup a positive transition alarm for when idle time exceeds 5000ms
3. Become idle, and wait for the alarms to trigger.
Expected result:
- When idle time exceeds 2000ms, the first alarm triggers.
- When idle time exceeds 5000ms, the second alarm triggers.
Actual result:
- When idle time exceeds 2000ms, the first alarm triggers.
- When idle time exceeds 5000ms, the second alarm does not trigger.
Analysis:
1) When you first setup the alarms, the SyncComputeBracketValues function is
called with startOver = TRUE. In this case, the bracket values are calculated
correctly, and the member variable "bracket_greater" on the idle time counter
is set to 2000.
2) When the first alarm triggers, the SyncComputeBracketValues function is
called with startOver = FALSE, from SyncChangeCounter. In this case, the second
alarm should be set up, and the member variable "bracket_greater" on the idle
time counter should be set to 5000. However, because the test value (5000) is
greater than the previous value of bracket_greater (2000), this alarm is not
enabled.
Solution:
To fix this problem, I updated the SyncChangeCounter function to call
SyncComputeBracketValues with startOver = TRUE. This causes "bracket_greater"
to be reset and the alarm to be triggered correctly. After doing this, the
startOver variable is always set to TRUE, so I also cleaned up the code and
eliminated this parameter altogether.
More information about the xorg-devel
mailing list