Merge lp:~mc-return/compiz/compiz.merge-fix1101549-and-fix1101561-missing-break into lp:compiz/0.9.9

[MC Return]

Fixed the cases MotionNotify == EnterNotify == LeaveNotify
in the AnnoScreen::handleEvent (XEvent *event) function.
Do not call handleMotionEvent (pointerX, pointerY) twice.
Fall-through here is intentional.
Added missing break to not fall-through to default.
Fixed indentation.

(LP: #1101549, LP: #1101561)

Note:
This fix has been tested and the test revealed no obvious changes in the behaviour
of Annotate.
There is still some flickering going on, during drawing of rectangle or ellipse, see
bug 1107818, this still has to be fixed. This did not regress with this MP, but was
there before.

[Sam Spilsbury (smspillaz)]

4 @@ -964,9 +964,12 @@
5 switch (event->type) {
6 case MotionNotify:
7 handleMotionEvent (pointerX, pointerY);
8 + break;
9 case EnterNotify:
10 + break;
11 case LeaveNotify:
12 handleMotionEvent (pointerX, pointerY);
13 + break;
14 default:

Ouch, an intended fall through caught by coverity as a false positive. I knew this would eventually happen.

Unfortunately this is currently broken (in a very small way). pointerX and pointerY are updated on enter leave and motion events however this code only handles the change on motion and leave.

I think coverity won't complain if you do this:

case EnterNotify:
case LeaveNotify:
case MotionNotify:
    handleMotionEvent (pointerX, pointerY);
    break;

review: Needs Fixing

[MC Return (mc-return)]

> 4 @@ -964,9 +964,12 @@
> 5 switch (event->type) {
> 6 case MotionNotify:
> 7 handleMotionEvent (pointerX, pointerY);
> 8 + break;
> 9 case EnterNotify:
> 10 + break;
> 11 case LeaveNotify:
> 12 handleMotionEvent (pointerX, pointerY);
> 13 + break;
> 14 default:
>
> Ouch, an intended fall through caught by coverity as a false positive. I knew
> this would eventually happen.
>
> Unfortunately this is currently broken (in a very small way). pointerX and
> pointerY are updated on enter leave and motion events however this code only
> handles the change on motion and leave.
>
> I think coverity won't complain if you do this:
>
> case EnterNotify:
> case LeaveNotify:
> case MotionNotify:
> handleMotionEvent (pointerX, pointerY);
> break;

Should it fall through to default as well or not ?
If not (which was the case previously) the last break
below handleMotionEvent (pointerX, pointerY); should
also be removed...

review: Needs Information

[MC Return (mc-return)]

Sam, default is this (if you do not have the code right now):

    default:
 if (event->type == cScreen->damageEvent () + XDamageNotify)
 {
     XDamageNotifyEvent *de = (XDamageNotifyEvent *) event;
     if (pixmap == de->drawable)
  cScreen->damageRegion (CompRegion (CompRect (de->area)));
 }
 break;
    }

[lp:~mc-return/compiz/compiz.merge-fix1101549-and-fix1101561-missing-break]

3586. By MC Return on 2013-01-28

Fall-through is intentional here:
case MotionNotify == EnterNotify == LeaveNotify

Fixed indentation

[Sam Spilsbury (smspillaz)]

Yep, you've got the right implementation.

Generally a fallthough to damage handling through default: is harmless, but putting the break statement there will make coverity shut up, which is probably a good thing.

review: Approve

[MC Return (mc-return)]

> Yep, you've got the right implementation.
>
> Generally a fallthough to damage handling through default: is harmless, but
> putting the break statement there will make coverity shut up, which is
> probably a good thing.

Oh, great. Thx 4 the review & the information :)

=== modified file 'plugins/annotate/src/annotate.cpp'
--- plugins/annotate/src/annotate.cpp	2012-09-04 15:33:44 +0000
+++ plugins/annotate/src/annotate.cpp	2013-01-28 13:48:20 +0000
@@ -961,12 +961,13 @@
 void
 AnnoScreen::handleEvent (XEvent      *event)
 {
-    switch (event->type) {
+    switch (event->type)
+    {
     case MotionNotify:
-	handleMotionEvent (pointerX, pointerY);
     case EnterNotify:
     case LeaveNotify:
 	handleMotionEvent (pointerX, pointerY);
+	break;
     default:
 	if (event->type == cScreen->damageEvent () + XDamageNotify)
 	{