Help reducing warnings in source code

Discuss the future of Warzone 2100 with us.
stiv
Warzone 2100 Team Member
Warzone 2100 Team Member
Posts: 876
Joined: 18 Jul 2008, 04:41
Location: 45N 86W

Re: Help reducing warnings in source code

Post by stiv »

Oh, ok, no help wanted, but it was a try
That is not what the man said. Some hints:

Break the patch into smaller pieces. Smaller patches that deal with one problem are easier to understand and more likely to get commited.

Don't run the code thru a beautifier. At least not as part of fixing another problem.

Just a quick glance, but creating assignment operators that don't do anything seems like a bad idea.
Seismo
Trained
Trained
Posts: 70
Joined: 06 Feb 2010, 17:32

Re: Help reducing warnings in source code

Post by Seismo »

Here we are, i break it to smaller pieces (lib sub folders and src folder) and eredicate all beauties (may one or two lines left). I hope that this could help a bit.

EDIT: new one here: viewtopic.php?f=6&t=4727&p=48772#p48772
Last edited by Seismo on 19 Feb 2010, 23:47, edited 3 times in total.
Seismo
Trained
Trained
Posts: 70
Joined: 06 Feb 2010, 17:32

Re: Help reducing warnings in source code

Post by Seismo »

continued

EDIT: new one here: viewtopic.php?f=6&t=4727&p=48772#p48772
Last edited by Seismo on 19 Feb 2010, 23:48, edited 1 time in total.
Seismo
Trained
Trained
Posts: 70
Joined: 06 Feb 2010, 17:32

Re: Help reducing warnings in source code

Post by Seismo »

continued

EDIT: new one here: viewtopic.php?f=6&t=4727&p=48772#p48772
Last edited by Seismo on 19 Feb 2010, 23:48, edited 1 time in total.
crux
Trained
Trained
Posts: 139
Joined: 16 Jan 2010, 03:21

Re: Help reducing warnings in source code

Post by crux »

correct me if I am wrong but I thought all patches should go on trac, not the forums?
I also looked at some of these patches and they contain style changes and not a cleanup of warnings
Per
Warzone 2100 Team Member
Warzone 2100 Team Member
Posts: 3780
Joined: 03 Aug 2006, 19:39

Re: Help reducing warnings in source code

Post by Per »

Yes, patches should go to trac, not here. They should also be split by type of change, and not by area of code they modify. These patches have warning fixes and code rewrites mixed up with completely senseless style changes like changing /* comments */ with // comments and re-indenting (not all of which follows CodingStyle).

Can you please resubmit the changes split by type of warnings fixed, and drop the cosmetic changes?
Seismo
Trained
Trained
Posts: 70
Joined: 06 Feb 2010, 17:32

Re: Help reducing warnings in source code

Post by Seismo »

Per wrote:Yes, patches should go to trac, not here.
I asked for, but did not get an answer.
Per wrote:They should also be split by type of change, and not by area of code they modify.
Put them in one and paint on it => "Reduce MSVC warnings in general".
Per wrote:These patches have warning fixes and code rewrites mixed up with completely senseless style changes like changing /* comments */ with // comments and re-indenting (not all of which follows CodingStyle).
Can you please resubmit the changes split by type of warnings fixed, and drop the cosmetic changes?
I am sorry folks, but i guess it is complete senseless to help you. It will take me longer to correct everything for each of you instead of just correcting these errors and warnings. I can swear, after i have split them again and erradicated the last three lines of "completely senseless style changes", the next one will jump out of the bush and help me to understand another tiny funny part which could be done better. It is seriously sad that you do not spend this energy in a better and a bit more stable hardened source code. :(
User avatar
Buginator
Professional
Professional
Posts: 3285
Joined: 04 Nov 2007, 02:20

Re: Help reducing warnings in source code

Post by Buginator »

Seismo wrote:I am sorry folks, but i guess it is complete senseless to help you. It will take me longer to correct everything for each of you instead of just correcting these errors and warnings. I can swear, after i have split them again and erradicated the last three lines of "completely senseless style changes", the next one will jump out of the bush and help me to understand another tiny funny part which could be done better. It is seriously sad that you do not spend this energy in a better and a bit more stable hardened source code. :(
Sorry, you feel that way, but cleaning up the codebase by using a beautifier screws everything up. If you read the ML, I wanted to do the same thing, a long time ago, then realized that it just wasn't worth it because all of the issues that will cause.

As Per said, we can take patches that add the casts (assuming they are correct ), and then we would run it through the other compilers we use to make sure it is OK, and then, finally, we would move them into the codebase.

This is how it is done with all patches that people send us.

We currently have quite a few branches (trunk, 2.3, Qt, lua ...) and we would run into tons of conflicts if we just applied those patches as is, and we don't have the manpower or the time to fix all that.

I hope this makes everything a bit more clear.
and it ends here.
Per
Warzone 2100 Team Member
Warzone 2100 Team Member
Posts: 3780
Joined: 03 Aug 2006, 19:39

Re: Help reducing warnings in source code

Post by Per »

Seismo wrote:I can swear, after i have split them again and erradicated the last three lines of "completely senseless style changes", the next one will jump out of the bush and help me to understand another tiny funny part which could be done better. It is seriously sad that you do not spend this energy in a better and a bit more stable hardened source code. :(
Well, except that from I look, we're all telling you the same thing, and as Buginator explained, it is for some very good reasons.

Also, adding casts all over the place does not result in a 'more stable hardened source code'. In fact, it is quite the opposite, as the code ends up more brittle when types are changed during development. The cast warnings should be turned off - implicit casts are allowed in C.
Seismo
Trained
Trained
Posts: 70
Joined: 06 Feb 2010, 17:32

Re: Help reducing warnings in source code

Post by Seismo »

Per wrote:Well, except that from I look, we're all telling you the same thing, and as Buginator explained, it is for some very good reasons.

Also, adding casts all over the place does not result in a 'more stable hardened source code'. In fact, it is quite the opposite, as the code ends up more brittle when types are changed during development. The cast warnings should be turned off - implicit casts are allowed in C.
Ok we are running in a circle. If you can see the serious warnings instantly between the 230 reported you might be much better than me. By correcting the warnings i also found unicode errors (WCHAR to CHAR casts) in exeption handler lib and a beauty misconfiguration in netplay which conflicts with global settings. You might not be able to see that because you are using a different operating system and/or a different compiler than me. I always mentioned that this would be a help for the MSVC users (wich also implicit that they use at least windows as operating system). I also took care not to come into conflict with other compilers and systems by using the appropriate preprocessor globals, due to the fact that i can not guarantee for things i do not use or have.
Samowar
Trained
Trained
Posts: 42
Joined: 03 Jun 2009, 19:46

Re: Help reducing warnings in source code

Post by Samowar »

Seismo wrote:If you can see the serious warnings instantly between the 230 reported you might be much better than me.
AFAIK, (nearly) all developers use gcc under Linux. There you have the option to turn certain sorts of warnings on and off via a compiler flag in the Makefile (as opposed to a pragma in the source code). Like lowering the warning level on MSVC, but more fine-grained. gcc with the current flags produces no warnings. And if warnings turn up, they're instantly corrected (standard cflags include -Werror - every warning counts as an error and immediately stops compiling).
Seismo
Trained
Trained
Posts: 70
Joined: 06 Feb 2010, 17:32

Re: Help reducing warnings in source code

Post by Seismo »

Why do we make a comparison between gcc/Linux(or Mac X) and MSVC/Windows here?
Per
Warzone 2100 Team Member
Warzone 2100 Team Member
Posts: 3780
Joined: 03 Aug 2006, 19:39

Re: Help reducing warnings in source code

Post by Per »

Can you post the 230 warnings (as attachment)?
Seismo
Trained
Trained
Posts: 70
Joined: 06 Feb 2010, 17:32

Re: Help reducing warnings in source code

Post by Seismo »

Here we are (its revision 9789).
You do not have the required permissions to view the files attached to this post.
Seismo
Trained
Trained
Posts: 70
Joined: 06 Feb 2010, 17:32

Re: Help reducing warnings in source code

Post by Seismo »

Ok, here is another try i would like to be annotated by you. I sorted the issues by warning number an a general pragma upgrade. I have done some changes to the vcproj files. I have done this patches with msvc 2008 (version 9.0) . A complete patch is also added if you want to save time with patching ;)
You do not have the required permissions to view the files attached to this post.