Re: [PATCH xserver 1/4] animcur: Use fixed-size screen private

[Aaron Plattner]

On 11/12/2017 04:35 PM, Robert Morell wrote:

On Mon, Nov 06, 2017 at 03:19:51PM -0500, Adam Jackson wrote:

Signed-off-by: Adam Jackson <a...@redhat.com>
---
  render/animcur.c | 15 ++++-----------
  1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/render/animcur.c b/render/animcur.c
index 52e6b8b79f..0707fe7271 100644
--- a/render/animcur.c
+++ b/render/animcur.c
@@ -77,12 +77,9 @@ static CursorBits animCursorBits = {

static DevPrivateKeyRec AnimCurScreenPrivateKeyRec; -#define AnimCurScreenPrivateKey (&AnimCurScreenPrivateKeyRec)

-
  #define IsAnimCur(c)      ((c) && ((c)->bits == &animCursorBits))
  #define GetAnimCur(c)     ((AnimCurPtr) ((((char *)(c) + CURSOR_REC_SIZE))))
-#define GetAnimCurScreen(s) 
((AnimCurScreenPtr)dixLookupPrivate(&(s)->devPrivates, AnimCurScreenPrivateKey))
-#define SetAnimCurScreen(s,p) dixSetPrivate(&(s)->devPrivates, 
AnimCurScreenPrivateKey, p)
+#define GetAnimCurScreen(s) ((AnimCurScreenPtr)dixLookupPrivate(&(s)->devPrivates, 
&AnimCurScreenPrivateKeyRec))

#define Wrap(as,s,elt,func) (((as)->elt = (s)->elt), (s)->elt = func)

  #define Unwrap(as,s,elt)    ((s)->elt = (as)->elt)
@@ -101,7 +98,6 @@ AnimCurCloseScreen(ScreenPtr pScreen)
      Unwrap(as, pScreen, RealizeCursor);
      Unwrap(as, pScreen, UnrealizeCursor);
      Unwrap(as, pScreen, RecolorCursor);
-    SetAnimCurScreen(pScreen, 0);
      ret = (*pScreen->CloseScreen) (pScreen);
      free(as);

The free() above should be removed as well, otherwise it causes a
double-free on screen teardown.

Other than that,

Reviewed-by: Robert Morell <rmor...@nvidia.com>
Tested-by: Robert Morell <rmor...@nvidia.com>

for the series.

At least two users have hit this now:
https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230799/#5230799

Adam, can this series be merged?

-- Aaron

Thanks,
Robert

      return ret;
@@ -308,15 +304,13 @@ AnimCurInit(ScreenPtr pScreen)
  {
      AnimCurScreenPtr as;

- if (!dixRegisterPrivateKey(&AnimCurScreenPrivateKeyRec, PRIVATE_SCREEN, 0))

+    if (!dixRegisterPrivateKey(&AnimCurScreenPrivateKeyRec, PRIVATE_SCREEN,
+                               sizeof(AnimCurScreenRec)))
          return FALSE;

- as = (AnimCurScreenPtr) malloc(sizeof(AnimCurScreenRec));

-    if (!as)
-        return FALSE;
+    as = GetAnimCurScreen(pScreen);
      as->timer = TimerSet(NULL, TimerAbsolute, 0, AnimCurTimerNotify, pScreen);
      if (!as->timer) {
-        free(as);
          return FALSE;
      }
      as->timer_set = FALSE;
@@ -329,7 +323,6 @@ AnimCurInit(ScreenPtr pScreen)
      Wrap(as, pScreen, RealizeCursor, AnimCurRealizeCursor);
      Wrap(as, pScreen, UnrealizeCursor, AnimCurUnrealizeCursor);
      Wrap(as, pScreen, RecolorCursor, AnimCurRecolorCursor);
-    SetAnimCurScreen(pScreen, as);
      return TRUE;
  }

--

2.14.2

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel