Merge lp:~mc-return/compiz/compiz.fix1030473-part3 into lp:compiz/0.9.8

[MC Return]

Reduced the scope of the variable 'icccm_version', removed a break that can never be executed, added the variables 'frameType', 'frameState' and 'frameActions' to decor_quads_to_property in the #else branch in KWD::Switcher::updateWindowProperties () as well and simplified calc_button_size (decor_t *d) in cairo.c.

Reduces the scope of the variable 'icccm_version', removes a break that can never be executed, adds the variables 'frameType', 'frameState' and 'frameActions' to decor_quads_to_property in the #else branch in KWD::Switcher::updateWindowProperties () as well and simplifies calc_button_size (decor_t *d) in cairo.c.

[Sam Spilsbury (smspillaz)]

8 - unsigned int nOffset = 1, frameType = 0, frameState = 0, frameActions = 0;
9 + unsigned int nOffset = 1, frameType, frameState, frameActions;

Are these guaranteed to be assigned to something we know later in the function? If so, we can move them there and declare and assign them at the same time.

[MC Return (mc-return)]

> 8 - unsigned int nOffset = 1, frameType = 0, frameState = 0,
> frameActions = 0;
> 9 + unsigned int nOffset = 1, frameType, frameState, frameActions;
>
> Are these guaranteed to be assigned to something we know later in the
> function? If so, we can move them there and declare and assign them at the
> same time.

Hmm. It seems I made a mistake - Maybe cppcheck had a problem with the #ifdef.
But those variables are used only here in the function updateWindowProperties ():

#ifdef QT_45
    decor_quads_to_property (data, nOffset - 1, mX11Pixmap,
        &mBorder, &mBorder, &mBorder, &mBorder,
        0, 0,
        quads, nQuad, frameType, frameState, frameActions);

So theoretically we could remove the declaration of those variables completely and simplify this to:

#ifdef QT_45
    decor_quads_to_property (data, nOffset - 1, mX11Pixmap,
        &mBorder, &mBorder, &mBorder, &mBorder,
        0, 0,
        quads, nQuad, 0, 0, 0);

...as those variables are not used anywhere else in this function and are always 0 anyway.

Unfortunately I am having problems convincing cmake and make to compile the KDE part of Compiz here to test it, but it *should* be okay. Please check the diff.

[lp:~mc-return/compiz/compiz.fix1030473-part3]

3305. By MC Return on 2012-08-05

Simplified updateWindowProperties () by removing redundant declaration of the variables 'frameType', 'frameState' and 'frameActions'

[MC Return (mc-return)]

Another question:

In compizconfig/gsettings/src/gsettings.c:296] cppcheck throws a warning: Casting between double* and float* which have an incompatible binary data representation

Investigation shows that this is correct, but it could be by design:
First we find this:

    case TypeFloat:
 {
     double *array = malloc (nItems * sizeof (double));

then in line 296:

     list = ccsGetValueListFromFloatArray ((float *) array, nItems, setting);

In compizconfig/libcompizconfig/include/ccs.h line 978 says:

CCSSettingValueList ccsGetValueListFromFloatArray (float *array,
         int num,
         CCSSetting *parent);

Q: Should we fix this and if so how ?

[MC Return (mc-return)]

Also in /gtk/window-decorator/cairo.c:732 in static void
calc_button_size (decor_t *d) we can find this:

    if (d->actions & (WNCK_WINDOW_ACTION_MINIMIZE |
        WNCK_WINDOW_ACTION_MINIMIZE))
 button_width += 17;

In the function static void calc_button_size (decor_t *d) button_width is first declared with

    gint button_width;

then assigned the value

    button_width = 0;

which could also be reduced to

    gint button_width = 0;

and then button_width is increased by 17 (pixels) for respective available window actions, so if the close action is valid, it is increased by 17 points, then for the maximize/restore button if any of the window actions for maximizing and unmaximizing are available, the third button would always be minimize then and we should fix this by changing:

    if (d->actions & (WNCK_WINDOW_ACTION_MINIMIZE |
        WNCK_WINDOW_ACTION_MINIMIZE))
 button_width += 17;

to

    if (d->actions & WNCK_WINDOW_ACTION_MINIMIZE)
 button_width += 17;

[lp:~mc-return/compiz/compiz.fix1030473-part3]

3306. By MC Return on 2012-08-05

Simplified calc_button_size (decor_t *d) in cairo.c

[MC Return (mc-return)]

cppcheck also warns about this in /compizconfig/libcompizconfig/src/main.c:4305: (error) Read and write operations without a call to a positioning function (fseek, fsetpos or rewind) or fflush inbetween result in undefined behaviour.

[lp:~mc-return/compiz/compiz.fix1030473-part3]

3307. By MC Return on 2012-08-05

Redeclared and reassigned frameType, frameState, frameActions unsigned int variables andalso added those to decor_quads_to_property in the #else branch in KWD::Switcher::updateWindowProperties () (switcher.cpp)

[Daniel van Vugt (vanvugt)]

Looks good, and passed smoke testing.

review: Approve

=== modified file 'gtk/window-decorator/cairo.c'
--- gtk/window-decorator/cairo.c	2012-06-22 06:11:26 +0000
+++ gtk/window-decorator/cairo.c	2012-08-05 14:28:18 +0000
@@ -716,9 +716,7 @@
 static void
 calc_button_size (decor_t *d)
 {
-    gint button_width;
-
-    button_width = 0;
+    gint button_width = 0;
 
     if (d->actions & WNCK_WINDOW_ACTION_CLOSE)
 	button_width += 17;
@@ -729,8 +727,7 @@
 		      WNCK_WINDOW_ACTION_UNMAXIMIZE_VERTICALLY))
 	button_width += 17;
 
-    if (d->actions & (WNCK_WINDOW_ACTION_MINIMIZE |
-		      WNCK_WINDOW_ACTION_MINIMIZE))
+    if (d->actions & WNCK_WINDOW_ACTION_MINIMIZE)
 	button_width += 17;
 
     if (button_width)

=== modified file 'kde/window-decorator-kde4/switcher.cpp'
--- kde/window-decorator-kde4/switcher.cpp	2011-05-07 08:58:43 +0000
+++ kde/window-decorator-kde4/switcher.cpp	2012-08-05 14:28:18 +0000
@@ -248,7 +248,7 @@
     decor_quads_to_property (data, nOffset - 1, mPixmap.handle (),
 			     &mBorder, &mBorder, &mBorder, &mBorder,
 			     0, 0,
-			     quads, nQuad);
+			     quads, nQuad, frameType, frameState, frameActions);
 #endif
     KWD::trapXError ();
     XChangeProperty (QX11Info::display (), mId, Atoms::netWindowDecor,

=== modified file 'libdecoration/decoration.c'
--- libdecoration/decoration.c	2012-05-10 15:40:25 +0000
+++ libdecoration/decoration.c	2012-08-05 14:28:18 +0000
@@ -3137,7 +3137,6 @@
 #define N_TARGETS 4
 
     Atom conversion_targets[N_TARGETS];
-    long icccm_version[] = { 2, 0 };
 
     conversion_targets[0] = XInternAtom (xdisplay, "TARGETS", 0);
     conversion_targets[1] = XInternAtom (xdisplay, "MULTIPLE", 0);
@@ -3153,9 +3152,12 @@
 			 XA_INTEGER, 32, PropModeReplace,
 			 (unsigned char *) &dm_sn_timestamp, 1);
     else if (target == conversion_targets[3])
+    {
+	long icccm_version[] = { 2, 0 };
 	XChangeProperty (xdisplay, w, property,
 			 XA_INTEGER, 32, PropModeReplace,
 			 (unsigned char *) icccm_version, 2);
+    }
     else
 	return 0;
 

=== modified file 'plugins/opengl/src/screen.cpp'
--- plugins/opengl/src/screen.cpp	2012-07-18 04:03:58 +0000
+++ plugins/opengl/src/screen.cpp	2012-08-05 14:28:18 +0000
@@ -656,7 +656,7 @@
 		    GLWindow::get (w)->priv->icons.clear ();
 	    }
 	    break;
-	break;
+
 	default:
 	    if (event->type == cScreen->damageEvent () + XDamageNotify)
 	    {