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.
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.
* highlighting.c: Bitfields in structs should properly have a full integer type.
* highlighting.c: Declaring these HighlightingStyle structures as "static" gets rid of "initializer element is not computable at load time" when they are used in the subsequent entries[] array.
* keybindings.c: Definition of keys[] moved here from keybindings.h. (See below for explanation)
* keybindings.c: Replaced the floating-point bit with a bit of integer math. This parameter is supposed to be an int, so you want to keep it an int.
* 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.
* 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!).
* sciwrappers.c: Argument 3 to SSM() is supposed to be unsigned, so stick in a cast to make it clear that what's getting passed is *not* -1.
* encodings.c: Moved the definition of the encodings[] array here from encodings.h.
* 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.)
* encodings.c: Use the handy-dandy printf specifier for gsize that GLib provides for us.
* encodings.h: Declare, don't define, the encodings[] array here.
* prefs.c: No comma after that last entry.
* dialogs.c: Cast the int to gboolean. (gboolean is probably int anyway, but an explicit cast makes things clearer.)
* msgwindow.c: Using the function parameters in the initializer gives a "initializer element is not computable at load time" warning, so just assign them to the fields afterward.
* keyfile.c: Compounds literals look cool, but they confuse older compilers.
* prefix.c: Stick in a trivial bit of code to avoid an empty source file when we're not building a relocatable binary.
* vte.c: Definition of vc moved here from vte.h.
* vte.h: Declare, don't define, the vc variable here.
* document.c: gchar is a signed type, so when you assign e.g. 0xef to it, GCC complains that the literal value overflows the type. So cast explicitly.
* plugins.c: The cast and extra parens in this conditional do not appear to be necessary.
* editor.c: event->x and event->y are gdouble, but sci_get_position_from_xy() wants gints, so cast appropriately.
* socket.c: Moved the definition of socket_info here from socket.h.
* socket.h: Declare, don't define, the socket_info struct here. Some extra footwork is needed here since the struct structure is being defined as well.
* tm_workspace.c: Got rid of a stray semicolon.
* 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.)
* sort.c: error() expects a formatted string + optional arguments, but since we're just passing a single, variable string, GCC gets nervous about not being able to check the format. Use a plain "%s" + string to set it at ease.
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.
--Daniel
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
On Tue, 19 Feb 2008 10:44:17 +0100 Enrico Tröger enrico.troeger@uvena.de wrote:
On Sun, 17 Feb 2008 00:43:56 -0500, "Daniel Richard G." skunk@iSKUNK.ORG wrote:
- (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?
Just because it's more typing and I had assumed it wasn't necessary. So I'm to blame ;-) I even removed them from any patches I applied, oops.
But I guess if it's not portable, we shouldn't use it.
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 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.
Regards, Nick
On Tue, 19 Feb 2008 14:08:28 +0000, Nick Treleaven nick.treleaven@btinternet.com wrote:
On Tue, 19 Feb 2008 10:44:17 +0100 Enrico Tröger enrico.troeger@uvena.de wrote:
On Sun, 17 Feb 2008 00:43:56 -0500, "Daniel Richard G." skunk@iSKUNK.ORG wrote:
- (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?
Just because it's more typing and I had assumed it wasn't necessary. So I'm to blame ;-) I even removed them from any patches I applied, oops.
But I guess if it's not portable, we shouldn't use it.
Ok, so we add them again.
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 was also thinking it causes warnings but no errors.
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.
I forgot the smiley ;-). I was trying to joke...:D. If we change it, we should really change it and not only fake it for the compiler.
Regards, Enrico
On Tue, 19 Feb 2008 16:27:46 +0100 Enrico Tröger enrico.troeger@uvena.de wrote:
On Tue, 19 Feb 2008 14:08:28 +0000, Nick Treleaven nick.treleaven@btinternet.com wrote:
On Tue, 19 Feb 2008 10:44:17 +0100 Enrico Tröger enrico.troeger@uvena.de wrote:
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.
I forgot the smiley ;-). I was trying to joke...:D. If we change it, we should really change it and not only fake it for the compiler.
Hehe, I get it now :) I probably take things too seriously.
Regards, Nick
On Sun, 17 Feb 2008 00:43:56 -0500 "Daniel Richard G." skunk@iSKUNK.ORG wrote:
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 :)
BTW do you use any special warnings for gcc that can reveal these things?
[...]
- highlighting.c: Bitfields in structs should properly have a full
integer type.
- highlighting.c: Declaring these HighlightingStyle structures as
"static" gets rid of "initializer element is not computable at load time" when they are used in the subsequent entries[] array.
I'll try to keep these in mind next time.
[...]
- 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.
- 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.
Regards, Nick
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
On Tue, 19 Feb 2008 13:41:16 -0500, "Daniel Richard G." skunk@iSKUNK.ORG wrote:
Hi,
I just applied the patch, thanks.
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.
Yes, I changed it.
- 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 }
Bahh, this is ugly. Even worse than having the additional line in a diff because of the comma.
Regards, Enrico
- 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 }Bahh, this is ugly. Even worse than having the additional line in a diff because of the comma.
How about something like this:
enum { FOO, BAR, BAZ, FBZ_LAST /* must be last*/ }
- Jeff
On Wed, 2008 Feb 20 06:53:51 -0600, Jeff Pohlmeyer wrote:
How about something like this:
enum { FOO, BAR, BAZ, FBZ_LAST /* must be last*/ }
That's a much nicer idea! It's often the case that the last item is e.g. FOO_COUNT, that can be referenced as the number of preceding enum elements. Having a convention for _LAST makes more sense when a total count is not relevant, say, for bitfield names or sparse enum lists.
I have another patch to submit, a fix for the as-of-recently multiply-defined symbol "c_tags_ignore". It should be declared in tagmanager/options.c as extern. (By the way, would it not be appropriate for this file to include symbols.h, so that it can get at the "official" declaration of that array?)
--Daniel
On Wed, 20 Feb 2008 17:22:12 -0500 "Daniel Richard G." skunk@iSKUNK.ORG wrote:
[...]
I have another patch to submit, a fix for the as-of-recently multiply-defined symbol "c_tags_ignore". It should be declared in tagmanager/options.c as extern. (By the way, would it not be
Thanks, applied.
appropriate for this file to include symbols.h, so that it can get at the "official" declaration of that array?)
Perhaps. Probably this was done because tagmanager was designed to be a separate library - although maybe it would be better if c_tags_ignore was actually defined in tagmanager rather than Geany.
Regards, Nick
On Fri, 22 Feb 2008 12:37:01 +0000, Nick Treleaven nick.treleaven@btinternet.com wrote:
appropriate for this file to include symbols.h, so that it can get at the "official" declaration of that array?)
Perhaps. Probably this was done because tagmanager was designed to be a separate library - although maybe it would be better if c_tags_ignore was actually defined in tagmanager rather than Geany.
Er, yes. When reading this it actually makes much more sense to have it as a tagmanager variable which is used by Geany, since tagmanager is the library. I guess this happened because I didn't intend to implement it in this way at first. It was just a try whether it works at all and then I decided to commit the code because it seemed to actually work ;-). I'm going to change it.
Regards, Enrico
On Tue, 19 Feb 2008 13:41:16 -0500 "Daniel Richard G." skunk@iSKUNK.ORG wrote:
[...]
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 ^_^
Thanks for the info. I'll try to write ANSI C89.
[...]
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.
Thanks, I'll update my flags soon.
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 [...]
Sounds useful, I'll keep it handy.
- 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).
OK, I guess it's probably not necessary for that amount of memory (probably only a few 100 bytes for typical users), and gboolean is clearer.
Regards, Nick
On Thu, 21 Feb 2008 16:58:07 +0000 Nick Treleaven nick.treleaven@btinternet.com wrote:
On Tue, 19 Feb 2008 13:41:16 -0500 "Daniel Richard G." skunk@iSKUNK.ORG wrote:
[...]
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.
Thanks, I'll update my flags soon.
I've tried the -pedantic option but I think I'll have to avoid that for my default CFLAGS until the C++-style comments are gone everywhere. This is because I like to use the -Werror option and there seems to be no way to turn off C++-style comment warnings, (because they are not part of ISO C90).
Still, if the Scintilla project agrees to this for their C headers I'll always use it. For now, I'll try to do some builds with -pedantic every now and then to catch portability problems.
Regards, Nick
Daniel Richard G. skunk@iskunk.org wrote:
Nick Treleaven wrote:
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
Thanks for the tip, Daniel - just for kicks I ran these flags on the GeanyLua plugin. ( Man, what a mess! :)
I managed to get everything cleaned up, with the exception of the -Wconversion flag. As far as I can tell, there is absolutely no way to use that flag with e.g. gtk_misc_set_alignment() without getting a warning.
So I did some googling[1] and it seems that the -Wconversion flag is not intended for general use, it is only useful when converting K&R code to ANSI/ISO. Apparently this issue has been addressed in gcc-4.3 but I'm still using 4.2.1
Just thought I would pass that along before someone else throws a keyboard through their monitor :-)
- Jeff
On Fri, 2008 Feb 22 08:19:44 -0600, Jeff Pohlmeyer wrote:
Thanks for the tip, Daniel - just for kicks I ran these flags on the GeanyLua plugin. ( Man, what a mess! :)
The first time is always the trickiest ^_^
I managed to get everything cleaned up, with the exception of the -Wconversion flag. As far as I can tell, there is absolutely no way to use that flag with e.g. gtk_misc_set_alignment() without getting a warning.
Well, there it would just be a matter of casting the (presumably integer) second and third arguments to float. Though I'd agree that doing this sort of thing every time the warning arises is overkill.
What I normally do is filter the warnings, and address only the conversions that are a bit iffy (float -> int, signed -> unsigned, etc.)---making up for the lack of granularity in what -Wconversion reports.
So I did some googling[1] and it seems that the -Wconversion flag is not intended for general use, it is only useful when converting K&R code to ANSI/ISO. Apparently this issue has been addressed in gcc-4.3 but I'm still using 4.2.1
Just thought I would pass that along before someone else throws a keyboard through their monitor :-)
Yeah, of the set of warnings I gave, that's the least compelling one. It's nice to use from the start with newly-written code, but in most other cases, it's not worth the effort to make the compiler shut up.
--Daniel
On Fri, Feb 22, 2008 at 12:27 PM, Daniel Richard G. skunk@iskunk.org wrote:
On Fri, 2008 Feb 22 08:19:44 -0600, Jeff Pohlmeyer wrote:
I managed to get everything cleaned up, with the exception of the -Wconversion flag. As far as I can tell, there is absolutely no way to use that flag with e.g. gtk_misc_set_alignment() without getting a warning.
Well, there it would just be a matter of casting the (presumably integer) second and third arguments to float. Though I'd agree that doing this sort of thing every time the warning arises is overkill.
Nope, even typecasting doesn't work. I don't think there is any possible way to workaround that warning.
For example, try compiling this with -Wconversion :
static void foo(float x) {
}
int main(int argc, char *argv[]) { foo((float) 0.0f); return 0; }
Crazy, isn't it?
- Jeff
On Fri, 2008 Feb 22 12:37:04 -0600, Jeff Pohlmeyer wrote:
Well, there it would just be a matter of casting the (presumably integer) second and third arguments to float. Though I'd agree that doing this sort of thing every time the warning arises is overkill.
Nope, even typecasting doesn't work. I don't think there is any possible way to workaround that warning.
For example, try compiling this with -Wconversion : [...] Crazy, isn't it?
Ah, I see what you mean. It must still be doing the implicit float->double type promotion... yeah, that's crazy :]
I usually filter it all into my warnings-summary script anyway, so it's not an overly big deal, but if I were running make(1) on the command line it would be preferable to drop the noise. So that real warnings don't slip through!
--Daniel