[Geany] [PATCH] Fixes for warnings and portability

[Daniel Richard G.]

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


-- 
NAME   = Daniel Richard G.       ##  Remember, skunks       _\|/_  meef?
EMAIL1 = skunk at iskunk.org        ##  don't smell bad---    (/o|o\) /
EMAIL2 = skunk at alum.mit.edu      ##  it's the people who   < (^),>
WWW    = http://www.******.org/  ##  annoy them that do!    /   \
--
(****** = site not yet online)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: warning-summary.pl
Type: text/x-perl
Size: 1031 bytes
Desc: not available
URL: <http://lists.geany.org/pipermail/users/attachments/20080219/36bdf65d/attachment.pl>