On Sun, 17 Feb 2008 00:43:56 -0500, "Daniel Richard G." skunk@iSKUNK.ORG wrote:
Hi,
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.
wow. Many, many thanks for this patch and your work do create and especially document it. Great work(the work we/I should have done in the past...).
(all missing parts from the original mail which I don't comment will be applied as they are)
Below is a walkthrough of the changes. I'll describe each sort of change only once, the first time it occurs in the patch file, for brevity:
- (various files): Anytime you have a prototype/definition of a function that doesn't take arguments, use (void) instead of (). This eliminates the "function declaration isn't a prototype" warning.
For some reason some time ago, we stopped using void in empty argument lists. Unfortunately, I don't remember why. Nick, do you?
- keybindings.h: Declare, don't define, the keys[] array here. Otherwise, you'll have multiple definitions of keys[] floating around (one for every .c file that uses this header), and the link step will break if you use -fno-common (which you should!).
Shame on me, this (and the others places where the variables are used this way) is old code from when I didn't know it better. Nick already told me about that and we started to change it, at least newer written code does it the right way using extern.
- encodings.c: i is unsigned, so cast that -1. (Enrico, you may want to have a closer look at the logic of this loop... it seems to assume that i might be (guint)-1 going in, but that's impossible, because (guint)-1 is not less than GEANY_ENCODINGS_MAX.)
Oops, this is crap. I first wrote the loop with a signed int and -1 was used to start the loop again from 0 (set i inside the loop to -1, at the next run i is increased to 0 and the loop starts again from the beginning, that was the idea). Later, I added the regex code and while doing this, I changed i to an unsigned int to re-use it above. Then I added the casts to guint to make gcc quiet but without deeper thinking about it. Thanks for doing my homework.
- encodings.c: Use the handy-dandy printf specifier for gsize that GLib provides for us.
Cool, didn't know that this exists.
- 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.
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.
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.
And again, many many thanks for this great work.
Regards, Enrico