Mir

Code review comment for lp:~dandrader/mir/switchingBundle_lp1270964

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

52 +/*
53 + Regression test for LP#1270964
54 + In the situation emulated here SwitchingBundle::last_consumed ise used without ever being set,
55 + thus its initial value of zero will wrongly be used and lead to bogus behavior.
56 + */
57 +TEST_F(SwitchingBundleTest, compositor_client_interleaved)

Would a better name be "does_not_hang_as_described_in_bug_1270964"?

71 + //(nbufs=3,fcomp=2,ncomp=1,fready=0,nready=1,fclient=1,nclient=1) <-- BUG starts here
72 + // Should be:
73 + //(nbufs=3,fcomp=0,ncomp=1,fready=0,nready=0,fclient=1,nclient=1)
...
77 + //(nbufs=3,fcomp=2,ncomp=1,fready=0,nready=2,fclient=2,nclient=0)
78 + // Should be:
79 + //(nbufs=3,fcomp=0,ncomp=1,fready=1,nready=1,fclient=2,nclient=0)

These comments are too cryptic for me.

81 + client_buffer = bundle.client_acquire(); // <- would lock here if in buggy state

I'm not a fan of tests that fail by hanging.

review: Needs Fixing

« Back to merge proposal