Subject: Re: bug#8410: make_invisible_cursor returns garbage if XCreateBitmapFromData fails

From: Jan Djärv <[email-masked]>
Date: Sat, 02 Apr 2011 08:45:06 +0200

It looks OK.  But there are lots of places in Emacs where errors and return 
values from X calls are not checked.  make_gc in the same file comes to mind.
If anything goes wrong there, emacs crashes on an X protocol error message.

If there is an error here it means the X server is out of memory, so Emacs 
will probably crash on another X call later on anyway.  But if you want to get 
rid of the gcc warning, your patch is correct.  If Emacs somehow survives
a bit longer, returning 0 means that the invisible cursor functionality will 
not work, the cursor will not become invisible.

	Jan D.

Paul Eggert skrev 2011-04-02 04.21:
> I found this bug via static analysis, using GCC 4.6.0's warnings.
> src/xfns.c's make_invisible_cursor returns a garbage value if
> XCreateBitmapFromData fails.  I plan to fix it as follows.
> I don't know that returning 0 fixes the bug, so I'd like someone
> who's expert in this area to look at this.  Since the patch replaces
> undefined behavior with defined behavior it isn't likely to be
> introducing a bug, so it shouldn't hurt to install the patch.
>
> * xfns.c (make_invisible_cursor): Don't return garbage
> if XCreateBitmapFromData fails.
> === modified file 'src/xfns.c'
> --- src/xfns.c	2011-04-01 20:30:45 +0000
> +++ src/xfns.c	2011-04-01 23:01:33 +0000
> @@ -855,19 +855,20 @@
>     static char const no_data[] = { 0 };
>     Pixmap pix;
>     XColor col;
> -  Cursor c;
> +  Cursor c = 0;
>
>     x_catch_errors (dpy);
>     pix = XCreateBitmapFromData (dpy, FRAME_X_DISPLAY_INFO (f)->root_window,
>                                  no_data, 1, 1);
>     if (! x_had_errors_p (dpy)&&  pix != None)
>       {
> +      Cursor pixc;
>         col.pixel = 0;
>         col.red = col.green = col.blue = 0;
>         col.flags = DoRed | DoGreen | DoBlue;
> -      c = XCreatePixmapCursor (dpy, pix, pix,&col,&col, 0, 0);
> -      if (x_had_errors_p (dpy) || c == None)
> -        c = 0;
> +      pixc = XCreatePixmapCursor (dpy, pix, pix,&col,&col, 0, 0);
> +      if (! x_had_errors_p (dpy)&&  pixc != None)
> +        c = pixc;
>         XFreePixmap (dpy, pix);
>       }
>
>
>
>