Skip to main content ↑ ↓
Topic: Patches to fix some warnings (Read 11107 times) previous topic - next topic

Patches to fix some warnings

2 little patches that fixes :
- some warnings with integer signed comparison
- some warnings with an enum handling

Re: Patches to fix some warnings

Reply #1 –
The first correction for cases is not really fixing warnings: Now other compiler are unhappy, that there is no return at the end of a function. Otherwise I agree with most of them.
 

Re: Patches to fix some warnings

Reply #2 –
It was just a quick fix since I didn't know if is was of some interest to you.

For the switch stuff i'll do as you did it, with a dbg statement.

Btw, for warnings like the one in gui/climates.cc:206 (there is a modification of the array that is a const), how should it be corrected ?
- With a const_cast<>()
- With a special setter() in the object ?

Re: Patches to fix some warnings

Reply #3 –
I do not get a warning in Visual C++ nor MinGW gcc 3.x, since there is a cast. Correcting warnings for some compilers is not really something I would do, since these may change again with next version. And (sint16 *) and const_cast<>() should do the same anyway.

However, the correct way is a function like access_climate_border() for setting this.

Re: Patches to fix some warnings

Reply #4 –
For me warnings are very useful to avoid programming mistakes (and therefore lurking bugs). I do think that it can sometimes severly impede the developpement  speed. When I am in the middle of something, I'm the first who doesn't like to be bothered with futile-just-right-now warnings. And that's why they are only warnings and not errors :-).
But after a while it's good to listen to our fellow compiler that makes all it can to help us...  

So, anyway, I don't mind cleaning up the compiler warnings, since I want to enter the codebase and that is a bite-sized task that might even be a little bit useful :-)

Btw, I use the stock gcc in debian etch (4.1.1).

And for the const_cast<>(), I do agree that it is the same as (sint16*)...

I'll submit more patches if you don't mind, just feel free to comment on them

Re: Patches to fix some warnings

Reply #5 –
Do not worry: Usually I am very critic with patches. ;) Unfourtunately only tron looks on my own works from time to time - thus I am sure there could be also a lot of things improved. And of course rectifying unsigned/signed mismatches is useful, since those could lead to conditions never met etc.

Re: Patches to fix some warnings

Reply #6 –
So, here is a grouped patch, it didn't have time to divide it in smaller items that could be atomic (sorry).

Re: Patches to fix some warnings

Reply #7 –
Please use standard patch. Your file forced me to rebuilt my local copy from scratch, since it could not be reversed nor checked in.

By the way from 191 warnings I got down to 179 ... a little to much for -Werror still with the MSVC compiler. Also BeOS still generates quite some warnings.

And the gui-texinput does not work properly. I could not rename a town with this.

EDIT: If you make gui_textinput.cc handling the text local, then you must notify caller with call_listener(text) and add handler to all routines which use this listener. This would be the proper way. (As the simutrans dialog system stems from three different kinds of dialog and input objects, this was my way of "quick and dirty" unify them (albeit quick was rather two months). In the current way I cannot apply your diff to the source.

Re: Patches to fix some warnings

Reply #8 –
Quote from: prissi – on
Please use standard patch. Your file forced me to rebuilt my local copy from scratch, since it could not be reversed nor checked in.

Sorry, i thought it was a standard GNU patch file since I could use it with :
Code: [Select]
patch -p0 < file.diff
(by global, i meant only one file, and not one for const, one for unused vars, etc. )

Quote from: prissi – on
a little to much for -Werror still with the MSVC compiler.

Yeah, I forgot to edit out the -Werror that shouldn't be in the release version anyway. It's only here to help me (or a dev guy) with warnings.

Quote from: prissi – on
By the way from 191 warnings I got down to 179
... Also BeOS still generates quite some warnings.

I'm quite interested in these, since i didn't know about them in my dev environment. If i can reproduce them, I might be able to propose a fix also.

Quote from: prissi – on
If you make gui_textinput.cc handling the text local, then you must notify caller with call_listener(text) and add handler to all routines which use this listener. This would be the proper way. (As the simutrans dialog system stems from three different kinds of dialog and input objects, this was my way of "quick and dirty" unify them (albeit quick was rather two months). In the current way I cannot apply your diff to the source.

Completely agreed on that : I don't understand the full architecture of simutrans yet. Therefore the patches I submitted are to be seen more as "work-in-progress" material than "ready-to-apply" patches.

Btw, thx for your time :-)

Re: Patches to fix some warnings

Reply #9 –
So, i'm trying to commit unit-sized patches :)

Here is one that says that a fps is unsigned.

Re: Patches to fix some warnings

Reply #10 –
Another bunch of patches

Re: Patches to fix some warnings

Reply #11 –
2 more..

Re: Patches to fix some warnings

Reply #12 –
destructor of spieler_t should be virtual:

Code: [Select]
=== simplay.h
==================================================================
--- simplay.h   (revision 338)
+++ simplay.h   (local)
@@ -195,7 +195,7 @@
         */
        spieler_t(karte_t *welt, uint8 player_nr );
 
-       ~spieler_t();
+       virtual ~spieler_t();
 
        static sint32 add_maintenance(spieler_t *sp, sint32 change) {
                if(sp) {

Re: Patches to fix some warnings

Reply #13 –
The whole system is getting overhauled. Since I do developing with MSVC (due to a better debugger and faster compiling) compilation on GCC might be broken. (I just started 10 minutes ago a testcompile and found out the same.) MSVC ignores this and makes it virtual anyway ... One of the nice stuff using two compilers. Still all AI will be for today only p****enger AIs.