Code review comment for lp://staging/~vanvugt/compiz/fix-1019337.2

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Also good,

23 if (result == Success && error == Success && data)

I don't think there's a reason to check for result and error being Success here - my understanding of the protocol is that XGetWindowProperty is a synchronous call and returns its error code (in this case being Success, BadValue, BadAtom or BadWindow) (http://www.x.org/docs/XProtocol/proto.pdf)). However, the error traps are necessary because we don't want the process to abort.

We should find other unguarded X Protocol calls which may also generate such errors too and guard them as well.

If it turns out that the actual XErrorEvent trapped may return something different to XGetWindowProperty then it's an Approve from me, otherwise remove the other redundant check and consider it Approved.

(No tests required in gtk-window-decorator now, there is no testing framework for it)

« Back to merge proposal