[PATCH 1/2] DRI2SwapBuffers: Don't reuse swap_target variable.

Jamey Sharp jamey at minilop.net
Fri Apr 18 12:22:18 PDT 2014

swap_target is an out-parameter that needs to be set to the value that
SBC will take on after this SwapBuffers request completes.

However, it was also being used as a temporary variable to hold the MSC
at which the SwapBuffers request got scheduled to occur. This confusion
makes it harder to reason about whether swap_target is being set
correctly for its out-parameter usage. (Hint: It isn't.)

For the latter use, it makes more sense to use the existing target_msc
variable, which already has the right value unless target_msc, divisor,
and remainder are all 0, in which case we can set it using swap_interval
as usual.

Signed-off-by: Jamey Sharp <jamey at minilop.net>
Signed-off-by: Theo Hill <Theo0x48 at gmail.com>
 hw/xfree86/dri2/dri2.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 729a323..962f40c 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -1149,17 +1149,13 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
          * we have to account for the current swap count, interval, and the
          * number of pending swaps.
-        *swap_target = pPriv->last_swap_target + pPriv->swap_interval;
+        target_msc = pPriv->last_swap_target + pPriv->swap_interval;
-    else {
-        /* glXSwapBuffersMscOML could have a 0 target_msc, honor it */
-        *swap_target = target_msc;
-    }
     ret = (*ds->ScheduleSwap) (client, pDraw, pDestBuffer, pSrcBuffer,
-                               swap_target, divisor, remainder, func, data);
+                               &target_msc, divisor, remainder, func, data);
     if (!ret) {
         pPriv->swapsPending--;  /* didn't schedule */
         xf86DrvMsg(pScreen->myNum, X_ERROR,
@@ -1167,7 +1163,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
         return BadDrawable;
-    pPriv->last_swap_target = *swap_target;
+    pPriv->last_swap_target = target_msc;
     /* According to spec, return expected swapbuffers count SBC after this swap
      * will complete.

More information about the xorg-devel mailing list