The International Simutrans Forum

 

Author Topic: Patches to fix some warnings  (Read 6731 times)

0 Members and 1 Guest are viewing this topic.

Offline snide

  • *
  • Posts: 10
Patches to fix some warnings
« on: August 22, 2008, 04:41:57 PM »
2 little patches that fixes :
- some warnings with integer signed comparison
- some warnings with an enum handling


Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10682
  • Languages: De,EN,JP
Re: Patches to fix some warnings
« Reply #1 on: August 22, 2008, 07:37:23 PM »
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.

Offline snide

  • *
  • Posts: 10
Re: Patches to fix some warnings
« Reply #2 on: August 22, 2008, 08:48:14 PM »
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 ?
« Last Edit: August 23, 2008, 12:19:05 PM by snide »

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10682
  • Languages: De,EN,JP
Re: Patches to fix some warnings
« Reply #3 on: August 23, 2008, 05:49:42 PM »
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.

Offline snide

  • *
  • Posts: 10
Re: Patches to fix some warnings
« Reply #4 on: August 24, 2008, 11:00:14 AM »
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

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10682
  • Languages: De,EN,JP
Re: Patches to fix some warnings
« Reply #5 on: August 24, 2008, 06:24:21 PM »
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.

Offline snide

  • *
  • Posts: 10
Re: Patches to fix some warnings
« Reply #6 on: August 28, 2008, 02:55:48 PM »
So, here is a grouped patch, it didn't have time to divide it in smaller items that could be atomic (sorry).

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10682
  • Languages: De,EN,JP
Re: Patches to fix some warnings
« Reply #7 on: August 28, 2008, 07:08:32 PM »
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.
« Last Edit: August 28, 2008, 07:41:47 PM by prissi »

Offline snide

  • *
  • Posts: 10
Re: Patches to fix some warnings
« Reply #8 on: August 29, 2008, 11:19:28 AM »
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. )

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.

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.

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 :-)

Offline snide

  • *
  • Posts: 10
Re: Patches to fix some warnings
« Reply #9 on: September 01, 2008, 03:28:37 PM »
So, i'm trying to commit unit-sized patches :)

Here is one that says that a fps is unsigned.

Offline snide

  • *
  • Posts: 10
Re: Patches to fix some warnings
« Reply #10 on: September 09, 2008, 06:35:07 PM »
Another bunch of patches

Offline snide

  • *
  • Posts: 10
Re: Patches to fix some warnings
« Reply #11 on: September 09, 2008, 06:35:30 PM »
2 more..

Offline snide

  • *
  • Posts: 10
Re: Patches to fix some warnings
« Reply #12 on: November 06, 2008, 09:50:41 PM »
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) {

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10682
  • Languages: De,EN,JP
Re: Patches to fix some warnings
« Reply #13 on: November 06, 2008, 10:17:27 PM »
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 passenger AIs.