xserver: Branch 'master' - 2 commits

[Keith Packard]

 hw/xfree86/dri2/dri2.c |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

New commits:
commit 138bf5ac9703b410a6066c303feea067680edf5a
Author: Jamey Sharp <jamey at minilop.net>
Date:   Fri Apr 18 12:22:19 2014 -0700

    DRI2SwapBuffers: Fix uninitialized target SBC.
    
    Fixes Piglit test "swapbuffersmsc-return swap_interval 0".
    
    Ensure that *swap_target gets initialized on any 'return Success' path,
    even if the swap request can't be completed by the driver and the server
    falls back to a simple blit. That path can also be triggered by setting
    swap_interval to 0, which disables sync to vertical retrace.
    
    We originally found this bug because for some reason SDL2 automatically
    sets swap_interval to 0, when we were trying to test OML_sync_control in
    an SDL2 test application. We then discovered that the above-mentioned
    Piglit test has been failing for the same reason since it was
    introduced.
    
    Signed-off-by: Jamey Sharp <jamey at minilop.net>
    Signed-off-by: Theo Hill <Theo0x48 at gmail.com>
    Reviewed-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Eric Anholt <eric at anholt.net>
    Signed-off-by: Keith Packard <keithp at keithp.com>

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 962f40c..76708ca 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -1092,6 +1092,14 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
         return BadDrawable;
     }
 
+    /* According to spec, return expected swapbuffers count SBC after this swap
+     * will complete. This is ignored unless we return Success, but it must be
+     * initialized on every path where we return Success or the caller will send
+     * an uninitialized value off the stack to the client. So let's initialize
+     * it as early as possible, just to be sure.
+     */
+    *swap_target = pPriv->swap_count + pPriv->swapsPending + 1;
+
     for (i = 0; i < pPriv->bufferCount; i++) {
         if (pPriv->buffers[i]->attachment == DRI2BufferFrontLeft)
             pDestBuffer = (DRI2BufferPtr) pPriv->buffers[i];
@@ -1165,11 +1173,6 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
 
     pPriv->last_swap_target = target_msc;
 
-    /* According to spec, return expected swapbuffers count SBC after this swap
-     * will complete.
-     */
-    *swap_target = pPriv->swap_count + pPriv->swapsPending;
-
     DRI2InvalidateDrawableAll(pDraw);
 
     return Success;
commit 4927af4475bc7d020190d9f468c90366525c1109
Author: Jamey Sharp <jamey at minilop.net>
Date:   Fri Apr 18 12:22:18 2014 -0700

    DRI2SwapBuffers: Don't reuse swap_target variable.
    
    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>
    Reviewed-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Eric Anholt <eric at anholt.net>
    Signed-off-by: Keith Packard <keithp at keithp.com>

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;
-    }
 
     pPriv->swapsPending++;
     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.