Responding to various:
On Tue, 2008 Feb 19 10:44:17 +0100, Enrico Tröger wrote:
- c.c: The cpp conditional should first check whether DEBUG_C is actually defined or not, before using it in a conditional expression. This change gets rid of the warning, but what you might want to do instead is just change the #if into an #ifdef. (I couldn't do that without knowing how DEBUG_C is used, so I leave that to you.)
This doesn't really matter. All debug cod in tagmanager isn't used at all. Or at least one have to manually define the DEBUG macros and then the code probably won't compile anymore or other problems may occur. In other words: just ignore everything inside any #ifdef DEBUG #endif contructs in all source files in the tagmanager directory.
Okay. You'll just want to change it to use #ifdef instead of #if, so that you don't get "DEBUG_C is not defined" warnings.
On Tue, 2008 Feb 19 14:08:28 +0000, Nick Treleaven wrote:
The biggest portability issue, however, is one that a patch is not well-suited to fix: the use of C++-style comments in C code. GCC allows this, but will readily warn that "C++ style comments are not allowed in ISO C90", and Unix compilers almost universally choke on them as a syntax error.
Oh noes ;-) Luckily I mainly use C-style comments. Again I had wrongly assumed that modern compilers would allow this.
I believe C++-style comments are allowed as of C99. In practice, however, C99 means either GCC, Intel's icc, or (maybe) newer versions of the Solaris/HP-UX/etc. C compiler than I've ever tried.
In my experience, ANSI C89 plus a few ubiquitous extensions (e.g. long long) is the sweet spot for portability, at least across most Unix platforms. If the platform can't handle ANSI C, it won't be able to build GTK+ anyway ^_^
I know and I know this since I'm using them ;-). The // comments are just nicer, shorter and easier to use. I'm just lazy :D. I agree it would be better to not use them. We could write a preprocessor macro to transform e.g. // some text here into /* some text here */ Hehe, this would make me and the compilers happy.
Hmm, I don't think you can do that with the C preprocessor, but probably you meant using a script.
If this is something you'd like to try, I have some Perl code that can do the bulk of the conversion. Single-line // comments are trivial; it's the multi-line comments, e.g.
// This is a comment. It // spans multiple lines, and // ends on this one.
that needs a less-straightforward conversion to
/* This is a comment. It * spans multiple lines, and * ends on this one. */
(I could also just send a patch, but it would be a biiiig patch...)
On Tue, 2008 Feb 19 14:14:39 +0000, Nick Treleaven wrote:
On Sun, 17 Feb 2008 00:43:56 -0500
I normally build software with many warnings turned on, and also frequently build on non-Linux Unix platforms. In the course of doing both with Geany, I came across many warnings and portability issues that the attached patch seeks to address.
As Enrico said, thanks :)
It great to improve a tool that one relies upon!
BTW do you use any special warnings for gcc that can reveal these things?
I have a standard set of flags:
CFLAGS = -O0 -g3 -pedantic -fno-common -pipe -W -Wall -Wcast-align \ -Wcast-qual -Wconversion -Wformat=2 -Winline -Wpointer-arith \ -Wshadow -Wundef -Wno-unused-parameter -Waggregate-return \ -Wnested-externs -Wstrict-prototypes
Some of those -W... flags may be too much, but at a mininum everything up to and including -Wall. I particularly recommend -fno-common; that one will catch multiply-defined symbols.
Oh, and I also use a custom script to quickly review all the warnings produced in a build. See attached. Give it a file containing the build output, warnings and all, and it'll output a nice list like
1314 passing argument N of 'blah' with different width due to prototype 406 C++ style comments are not allowed in ISO C90 399 missing initializer for member 'Foo::bar' 300 passing argument N of 'blah' as floating rather than integer due to prototype 170 passing argument N of 'blah' as 'float' rather than 'double' due to prototype 118 passing argument N of 'blah' as unsigned due to prototype 66 passing argument N of 'blah' as signed due to prototype 47 cast discards qualifiers from pointer target type [...]
- highlighting.h: The "gboolean" type already indicates that these
fields are true-false flags, and GCC complains about portability if a bitfield specifier is used.
OK, maybe I'll change them to integer types so they can share a DWord with bitfields.
Is there a need for bitfields here? I mean, it's nice to have a true/false value occupy one bit instead of an int (gboolean), but the code is a lot cleaner with the latter approach. You rarely see bitfields outside of device/system code, and places where you really need to do bit-packing (e.g. a struct that lets you get at the parts of an IEEE float).
- prefs.c: No comma after that last entry.
Oops, I recently started doing that to avoid the extra comma change in diffs when appending items. Ah well, another thing to leave out until I'm D programming.
Good point, about the diffs. I suppose you could do something like
enum { FOO , BAR , BAZ }
taking a page from typical Yacc syntax....
--Daniel