Re: Deskguide crash bug - FIXED (I hope)

[otaylor fresnel labs redhat com]

Peter Wainwright <prw wainpr demon co uk> writes:

> Package: gnome-core
> 
> It has recently been reported that the deskguide applit in the HEAD branch of
> gnome-core sometimes crashes. This seems to occur when it attempts to draw a 
> thumbnail of a window which is being closed. So, I compiled a version with
> -g and waited for a crash. When I examined the core, I found a NULL
> pointer dereference. The following patch should fix this one (there
> may be many others but they haven't been discarvard - Tom Lehrer :-)).


> @@ -286,7 +287,7 @@
>    /* fallback to root window's colormap */
>    if (!cmap)
>      {
> -      GdkVisual *visual = gdk_window_get_visual (drawable);
> +      visual = gdk_window_get_visual (drawable);
>  
>        if (visual && visual->depth == gdk_window_get_visual (NULL)->depth)
>  	cmap = gdk_window_get_colormap (NULL);
> @@ -298,7 +299,10 @@
>    /* for depth matches, we first try to grab the image using our global
>     * shared memory pixmap image
>     */
> -  if (shm_image && shm_image->depth == gdk_window_get_visual
> (drawable)->depth)+  visual = gdk_window_get_visual (drawable);
> +  if (!visual)		/* screwed again - maybe window is closing? */
> +    return NULL;
> +  if (shm_image && shm_image->depth == visual->depth)
>      {
>        GdkGCValues gc_values;
>        GdkGC *gc;

A simpler and more reliable way to get the depth is:

  gdk_window_get_geometry (window, NULL, NULL, NULL, NULL, &depth);

Of course, the real thing here to note is that using GDK in
conjunction with ignoring errors is not reliable since GDK does
not always check the result of functions that return a Status.

That is, when ignoring X errors, you can do:

  if (XGetGeometry (display, drawable, root, &x, &y, &w, &h, &border, &depth))
    {
       [ use depth ] 
    }

But if you want to go through GDK (and I don't think there is really
much point here - this code is not one whit portable), you need to do:

 gdk_error_trap_push ();
 gdk_window_get_geometry (window, NULL, NULL, NULL, NULL, &depth);
 if (gdk_error_trap_pop () == 0)
   {
     [ Use depth ]
   } 

I haven't examined the desk-guide code, so this is just a comment
on the snippet of code aboive.

Regards,
                                        Owen