Bug 7963

Created attachment 6653 [details] [review]
cleanups

Kjartan,

In general the cleanups look good. Could you please separate this patch into
individual patches based on the class of issue being resolved? It looks like
there are about three separate things happening in the current patch.

Of course, each separate patch should also have its own description for the
commit message.

-Carl
 

Also check:

  http://lists.freedesktop.org/archives/cairo/2006-August/007626.html

Created attachment 7617 [details] [review]
patch to remove unsused code

Created attachment 7618 [details] [review]
if vs ifdef changes

looks like this overlaps with pavel's work, and that it's not wanted anyway.
putting it here for completeness

Ping? I think these still apply.

Ok, committed the unused-code patch.

About the #if syntax one, I'm a bit hesitant, given the current mixed usage numbers:

[behdad@home src]$ grep '^#if\>' *.c | wc -l
63
[behdad@home src]$ grep '^#if\>' *.h | wc -l
39
[behdad@home src]$ grep '^#if[dn]' *.c | wc -l
49
[behdad@home src]$ grep '^#if[dn]' *.h | wc -l
69

Any reason to only change the ones you are doing in the patch?  How are they
different?

The debate over '#if' vs '#ifdef' will flare up every now and again without any victors or pressing reason to change all the instances in the code. That said, the code is gradually become more consistent in its use of '#if'.

Cleanups from sparse and other static analysers are always desired. However with sparse we first need a method to increase the signal to noise, and then teach it how to handle the rules peculiar to cairo. So instead we're looking at other tools such as http://developer.mozilla.org/en/Treehydra which currently look more promising.

Closing as this bug as they are no more specific actions required. But don't let that stop you from continuing to test cairo and report as many warning as you find! (Patches preferred ;-)

I now have a test in src/ making sure that CAIRO_* conditionals are never used with ifdef.