Code review comment for lp:~smspillaz/compiz/compiz.merge-fix771448-desktop-wall-edge-flipping-broken.wall_edge_flip_logic

Can you verify it please?

On Sat, Apr 27, 2013 at 6:06 PM, MC Return <email address hidden> wrote:
> Review: Approve
>
>> In testing I remember coming across something that would effectively
>> cause actions to be removed from the active actions list multiple
>> times, which would in term cause the screen edge reference count to go
>> below zero. The if condition was the problem. It read:
>>
>> if NOT
>> (privateScreen.initialized OR
>> action->active ())
>> return false;
>>
>> The sub-condition is true if either privateScreen.initialized or
>> action->active () is true. This means that in order for it to be
>> false, both of them must be false. Eg,
>>
>> !(A || B) -> !A && !B
>>
>> That's incorrect, because privateScreen.initialized is usually always
>> true, so you'd run into situations like this:
>>
>> A && !B
>>
>> But that still satisfies A || B because A is still true.
>>
>> The adjusted condition is correct:
>>
>> if (!A || !B) -> !(A && B)
>>
>> What we want is for the function to always return false if
>> action->active () is false, regardless of what the value of
>> privateScreen.initialized is.
>>
>>
> Aha -> I thought this might be the problem, but was too lazy to really rethink
> the boolean logic (I am still missing my shower & coffee today ;))...
>
> I will approve this blindly this time, because I hope and am sure you will
> immediately take care of regressions should those occur...
>
>
> --
> https://code.launchpad.net/~smspillaz/compiz/compiz.merge-fix771448-desktop-wall-edge-flipping-broken.wall_edge_flip_logic/+merge/161279
> You are the owner of lp:~smspillaz/compiz/compiz.merge-fix771448-desktop-wall-edge-flipping-broken.wall_edge_flip_logic.

--
Sam Spilsbury