Mir

Code review comment for lp:~mzanetti/mir/dont-crash-when-shooting-invalid-surface

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I feel this syntax does not scale well:
   msh::Snapshot{{},{},nullptr}
It would probably be nicer to declare a const null_snapshot or something. But it's no big deal.

Kevin: I don't think throwing an exception is helpful. If the default_surface can be null under normal circumstances then I think take_snapshot should succeed transparently as in this proposal.

Also, there would seem to be a race between default_surface and take_snapshot_of. I'm assuming the only reason it's not racing is because surface is a shared_ptr.

Finally, I agree if the bug is simply crashing then it's a good candidate for an automated test case. But won't block on that. We should always welcome contributions from outside the team :)

review: Approve

« Back to merge proposal