crash after setting root background to None, then setting color - patch for review

[walter harms]

Looks ok to me, feel free to improve the code further,

Reviewed-by: Walter Harms


Am 17.05.2011 20:22, schrieb Marko Macek:
> Hello!
> 
> Please review/apply this patch:
> 
> diff -ru xorg-server-1.10.1/dix/window.c
> xorg-server-1.10.1-new/dix/window.c
> --- xorg-server-1.10.1/dix/window.c    2011-02-25 04:27:25.000000000 +0100
> +++ xorg-server-1.10.1-new/dix/window.c    2011-05-15 15:15:03.912919996
> +0200
> @@ -474,6 +474,7 @@
>          pWin->background.pixel = pScreen->whitePixel;
>          backFlag |= CWBackPixmap;
>      } else {
> +    pWin->backgroundState = BackgroundPixel;
>      if (whiteRoot)
>              pWin->background.pixel = pScreen->whitePixel;
>          else
> @@ -972,6 +973,7 @@
>      else if (party_like_its_1989)
>      MakeRootTile(pWin);
>      else {
> +    pWin->backgroundState = BackgroundPixel;
>      if (whiteRoot)
>          pWin->background.pixel = pScreen->whitePixel;
>      else
> 
> 
> This is valgrind output:
> 
> ==2514== Process terminating with default action of signal 11 (SIGSEGV)
> ==2514==  General Protection Fault
> ==2514==    at 0x37A60F5185: __sprintf_chk (sprintf_chk.c:28)
> ==2514==    by 0x4616B4: LogVWrite (stdio2.h:34)
> ==2514==    by 0x4613EA: ErrorF (log.c:561)
> ==2514==    by 0x4A13CD: xorg_backtrace (backtrace.c:44)
> ==2514==    by 0x4622C5: OsSigHandler (osinit.c:136)
> ==2514==    by 0x37A640F3BF: ??? (in /lib64/libpthread-2.13.90.so)
> ==2514==    by 0x4B88BC: ShmDestroyPixmap (privates.h:158)
> ==2514==    by 0x4573E2: ChangeWindowAttributes (window.c:1086)
> ==2514==    by 0x42923F: ProcChangeWindowAttributes (dispatch.c:692)
> ==2514==    by 0x42EC10: Dispatch (dispatch.c:431)
> ==2514==    by 0x422E19: main (main.c:287)
> ==2514==
> 
> The situation that causes an xserver crash is this:
> 
> 1. XSetWindowBackground(..., root, None)
> 2. followed by XSetWindowBackground(...)
> 
> 
> The code could also be made more robust (check for pixmap != None, set
> it to None after Destroy). Should I do it?
> 
>     Mark
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>