Using gcc on AIX I do get these compiler error: /.../cairo-9999/util/cairo-script/cairo-script-scanner.c: In function '_buffer_grow': /.../cairo-9999/util/cairo-script/cairo-script-scanner.c:200: error: 'csi_scanner_t' has no member named '__jmpbuf' /.../cairo-9999/util/cairo-script/cairo-script-scanner.c:206: error: 'csi_scanner_t' has no member named '__jmpbuf' Problem here is that *) cairo-script-scanner.c does #include "cairo-script-private.h" *) "cairo-script-private.h" does #include "config.h" #include <setjmp.h> typedef struct _csi_scanner csi_scanner_t; struct _csi_scanner { jmp_buf jmpbuf; ...} *) cairo-script-scanner.c does #include <math.h> *) <math.h> does #include <stdlib.h> *) <stdlib.h> does #include <sys/wait.h> *) <sys/wait.h> does #include <sys/signal.h> *) <sys/signal.h> does #include <sys/context.h> *) <sys/context.h> does #define jmpbuf __jmpbuf *) cairo-script-scanner.c does use: _csi_scanner.jmpbuf which is now defined to: _csi_scanner.__jmpbuf but was declared above as: _csi_scanner.jmpbuf
Created attachment 113847 [details] [review] proposed patch for proper include order This proposed patch does: for cairo-script-private.h: *) remove include "config.h" for cairo-script-*.c: 1) include "config.h" for the ABI-specific macros, before anything else 2) include system headers 3) include local headers
In studying bug 89354, and googling around for best practices in order of header inclusion, there seems to be a rough concensus[1] that you should include in this order: Given foo.c: 1. "config.h" 2. "foo.h" 3. headers from the same component 4. headers from other components 5. system headers The rationale here is to avoid hidden dependencies between headers. For comparison, Google's coding style guide[2] swaps #3 and #5, arguing that this ensures that build breaks show up first for people working on the files in question, not for people in other projects. Poking around Cairo .c files, the first form seems to be the more common convention. I think in the interest of consistency we should stick with that form. But looking at the original problem, maybe header shuffling is the wrong way to solve this. The actual underlying problem here is a namespace conflict on jmpbuf. The _csi_scanner structure is not part of any public API, so how about just renaming the member to jump_buffer? [1] http://stackoverflow.com/questions/2762568/c-c-include-file-order-best-practices [2] http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Order_of_Includes
(In reply to Bryce Harrington from comment #2) > In studying bug 89354, and googling around for best practices in order of > header inclusion, there seems to be a rough concensus[1] that you should > include in this order: As long as "config.h" is included first, I don't really care which practice a particular project does prefer. > But looking at the original problem, maybe header shuffling is the wrong way > to solve this. The actual underlying problem here is a namespace conflict > on jmpbuf. The _csi_scanner structure is not part of any public API, so how > about just renaming the member to jump_buffer? Renaming is fine with me too.
commit b4a922f62d34973ea89495b40ce8bc6378110b9e Author: Bryce Harrington <bryce@osg.samsung.com> AuthorDate: Tue Jun 16 16:42:56 2015 -0700 Commit: Bryce Harrington <bryce@osg.samsung.com> CommitDate: Thu Jun 18 12:38:59 2015 -0700 cairo-script: Rename struct member to avoid name collision on AIX On AIX, the token jmpbuf is a pre-processor macro. cairo-script-scanner.c includes a private struct with a member named jmpbuf which gets renamed to __jmpbuf when AIX's sys/context.h has been included. While judicious ordering of includes might kludge around this problem (by causing all references to .jmpbuf to become .__jmpbuf), it's better to simply select a new name for the struct member that won't suffer the collision. Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=89339 Signed-off-by: Bryce Harrington <bryce@osg.samsung.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.