Warnings

[Lighta]

So this is kinda old stuff but I think we should take more in accounts warnings and if possible fix them.

Currently we only have fews build who throwing out warnings : (http://build.rathena...ings (40)]NMAKE[/url]) cause other one are either set as silent or don't got a warnings level sufficient.

I'm suggesting to change visual warnings rom lvl 3 to lvl 4 and modify configure to remove -Wno parenthese, -Wno unused -Wno-sign-compare -Wno-pointer-sign.

For configure since we trying to optimise and make the code stronger those silent warnings serve againt our purpose, Why do we need to silent parenthese, bettter to let us know if a condition is dubious : if(a && b || c) => do you mean ((a && || c) or (a && (b || c)). unused is trivial and worse is done in Vcc but not in Gcc cause we tell him to shut us, what the logic in that ?

The only one I believe should stay is -Wno switch cause we using lot of switch and enum, and he will complaint to not have them all, well we could always put a default path for all of them.

If there still some warnings and you feel we should silent them, let at least do it if degug mode is off (--enable debug) so we can easely take a look at it later.

Of course I also believe buildbot should have debug mode on for compile so we know what missing.

Configure diff

Vcc Files

If you doing this you'll see those kind of warnings :

http://upaste.me/e1dc170363199020

So quick study of them we assigning to smaller type a bigger type like :

short a = int b, so ofc only half of b will be assign.

Signed to unsigned or unsigned to signed

short a = unsigned short b;

unsigned short a = short a;

So ofc after SHRT_MAX will have some issue of sign

nonstandard extension used : bit field types

char a:1; unsigned short a:1;

All useless we said we wanted only 1 bit assign for this type, putting short or char is useless as we still only gonna have 1bit

This only bring confusion as expected type, real declaration is : int a:x (x = number of bit)

etc...

So just changing those ain't hat hard and will put the code more coherent, they were mostly done by optimisation attempt, like we change mapid to short but not in function call causing those issues or trought scanf...

This will also optimise the code cause we will reduce in most case type so less ram reserved.

I already starting doing the change, ain't that easy to explain rule change but it's basically :

-1) keep struct assign in type if optimise;

-2) harmonize other struct we have the same index in there.

-3) Update local variable for correct match

-4) Update function parameter call

-5) cast

Here what I've done for the moment : ra_warns2.diff

So tell me what u think about it

Edited December 4, 2012 by Lighta
updated diff

[xazax]

Maybe it is just me, but I prefer to only do one specific refactoring with one commit.

For example to turn on one warning in gcc and fix those warnings with the same commit may be a bit easier to track in the future than huge changesets.

For the same reason it may be better to first enable several gcc warning and only set w4 warning level in visual studio after the warnings in gcc are fixed.

[Lighta]

If I do turn them on without fix this would generate lot of warnings for people.

Well I can put tjem under debug mode only for gcc but I've got no clue how to set that for VCC.

I'd also prefer to not have a huge changeset but this may be complicate for this one, well I sure can do 1 for refactoring then the next one following for type mistmatch fix wich would reduce the amount but the one for refactoring people may not like it since it won't bring any interesting change except that it's easier for us to follow those type.

(eg : one refactoring in here is skill_num, skillid, skillnum even sometime skill => skillid)

They all represent skill_id but was named differently, not such a big deal but it's easier to track skill_id with the same name then search for those other type, after that I'll put them all to same type with one big change. (currently some are ushort, uint, int, ...) wich is not safe for overflow case could be optimize easily.

[xazax]

Well, HUGE +1 to have uniform names for variables.