[MPlayer-dev-eng] [PATCH][TRIVIAL] Few cosmetics to x11_common

[Reimar Döffinger]

On Tue, Nov 03, 2009 at 06:10:29PM -0300, Reynaldo H. Verdejo Pinochet wrote:
> Hello Again
> 
> Reimar Döffinger wrote:
> > 
> > That one is of course fine, as well as quite a few other
> > changes.
> 
> Just thinking out loud here. Do you like these? I usually
> surround if/while statements with blanks.

Not really, the if is easy enough to see due to the indentation
IMO, but I don't care much.

> Index: x11_common.c
> ===================================================================
> --- x11_common.c        (revision 29818)
> +++ x11_common.c        (working copy)
> @@ -189,16 +189,18 @@
>          return;                 // do not hide if playing on the root
> window
> 
>      colormap = DefaultColormap(disp, DefaultScreen(disp));
> +
>      if ( !XAllocNamedColor(disp, colormap, "black", &black, &dummy) )
> -    {
>        return; // color alloc failed, give up
> -    }
> +
>      bm_no = XCreateBitmapFromData(disp, win, bm_no_data, 8, 8);
>      no_ptr = XCreatePixmapCursor(disp, bm_no, bm_no, &black, &black, 0, 0);
>      XDefineCursor(disp, win, no_ptr);
>      XFreeCursor(disp, no_ptr);
> +
>      if (bm_no != None)
>          XFreePixmap(disp, bm_no);
> +
>      XFreeColors(disp,colormap,&black.pixel,1,0);

Also e.g. here the colormap is only used to allocate/free the color.
It's all not that important, but if I was coding it from scratch I'd
probably go for an arrangement like

>      colormap = DefaultColormap(disp, DefaultScreen(disp));
>      if ( !XAllocNamedColor(disp, colormap, "black", &black, &dummy) )
>        return; // color alloc failed, give up
>      bm_no = XCreateBitmapFromData(disp, win, bm_no_data, 8, 8);

>      no_ptr = XCreatePixmapCursor(disp, bm_no, bm_no, &black, &black, 0, 0);
>      XDefineCursor(disp, win, no_ptr);
>      XFreeCursor(disp, no_ptr);

>      if (bm_no != None)
>          XFreePixmap(disp, bm_no);
>      XFreeColors(disp,colormap,&black.pixel,1,0);

I.e. split it into setup of "helper data", the cursor (our real goal) creation
and setting, and freeing the "helper data".
But this is of course a bikeshed issue...