Code review comment for lp:~compiz-team/compiz/fix-1060804

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

Okay, looking better.

The fact that C does not let you forward-declare typedef's is a bit of a pain. Right now in order to avoid massive header proliferation, I've just moved all of the typedefs to one header file per logical "package". This means that any time you add a new type, lots of stuff is going to be recompiled, which sucks, but its better than pulling in headers for types and functions and then lots of stuff recompiling when a function changes (even when that function isn't used).

It turns out that the stuff marked __attribute__((unusued)) was meant to be deleted ages ago. Its effectively duplicated in the .cpp files.

I'm still not sure if it even runs with clang. Last time I ran it, I had trouble with a bug similar to this one: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44059 . Maybe we should add a testcase to check if that bug exists?

review: Approve

« Back to merge proposal