On Wed, 24 Mar 2010 at 23:00:54 +0200, Johann Haarhoff wrote:
> 
> Subject: [PATCH] Port WM2 branch onto next

> --- a/WINGs/wwindow.c
> +++ b/WINGs/wwindow.c
> @@ -213,42 +213,35 @@ static void setMiniwindowTitle(WMWindow * win, const 
> char *title)
>                       PropModeReplace, (unsigned char *)title, strlen(title));
>  }
>  
> -static void setMiniwindow(WMWindow *win, RImage *image)
> +static void setMiniwindow(WMWindow *win, cairo_surface_t *image)
>  {
> -     WMScreen *scr = win->view->screen;
> -     long *data;
> +     WMScreen *scr= win->view->screen;
> +     CARD32 *data;

*data should not be CARD32, but 'long' as the original version.
There are problems in using CARD32 for a 'long' in 64-bits.

The original cairo work from Alfredo and Dan probably was using CARD32
because the patch removing this particular CARD32 is from Sept 2009,
see commit c7f2a189.


> -     data = wmalloc((image->width * image->height + 2) * sizeof(long));

> +     data= wmalloc((w*h+2)*sizeof(CARD32));

I realize it is too early to complain about the style, but as this
line will have to be edited anyway because of CARD32, it should be

+       data= wmalloc((w * h + 2) * sizeof(long));

i.e. there must be spaces between math operators.

  
>       XChangeProperty(scr->display, win->view->window, scr->netwmIcon,
>                       XA_CARDINAL, 32, PropModeReplace,
> -                     (unsigned char *)data, (image->width * image->height + 
> 2));
> +                     (unsigned char *)data,
> +                     (w * h + 2));

Why the new line for (w * h + 2)? It fits in the above and would make the
patch more readable.

> -void WMSetWindowMiniwindowImage(WMWindow * win, RImage * image)
> +void WMSetWindowMiniwindowImage(WMWindow * win, cairo_surface_t * image)

May I suggest using (WMWindow *win, cairo_surface_t *image) while you
are at it?

This bug in the style is due to my automatic convertion to the linux
kernel style using 'indent' a while ago, where 'indent' would use
the correct style (without space between * and the name) if the type
was a known C type like 'char', 'int' etc, but used the wrong style
for "unknown" types like WMWindow above :-(

Again, I am sorry for this. But as you are changing the lines anyway,
I thought it would be better to point it out.

Do you want me to fix the these or have you already made more changes
to the code in the meantime? 

In fact, this is something I want to ask you. What workflow would you 
prefer right now? If somebody decides to make changes (like the CARD32 
above) should it go through you or will you be able to cope with the 
code changing behind your back in #cairo, with you having probably 
changes locally in your tree and having to rebase your own work locally
to take into account the "moving" cairo branch?


-- 
To unsubscribe, send mail to [email protected].

Reply via email to