[Geany] [PATCH] Fixes for warnings and portability

[Enrico Tröger]

On Sun, 17 Feb 2008 00:43:56 -0500, "Daniel Richard G."
<skunk at 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

-- 
Get my GPG key from http://www.uvena.de/pub.key
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.geany.org/pipermail/users/attachments/20080219/d4f67648/attachment.pgp>