Jump to content
Eternal Lands Official Forums
ttlanhil

currently cleaning up warnings in CVS

Recommended Posts

this is just a heads-up, so that anyone who's working on parts of the code can tell me to wait a bit for whichever file...

 

I'm currently going though ELC to reduce/remove warnings... so far there have been some 64bit issues, but mostly signedness issues...

in some cases, it's fair enough. for example, a lot of the XML library stuff uses xmlChar.

in other cases, it's not so sensical... using Uint8 when char should be used, stuff like that

 

all edits are in-place, the only changes being made are to function definitions or where the warning occurs, so people shouldn't have any problems unless they're currently editing in the same place as a warning pops up.

 

it'll take me a little while to go through them all and fix or suppress as appropriate, but it shouldn't take that long... hopefully after this is done, building with all warnings on will be more common, and it'll be easy to see warnings in edited code

Share this post


Link to post
Share on other sites

okay, finished going through it, at least for my set of defines (and no, I don't care to do it for every combination ;) )

 

quite a few times, a daunting few pages of warnings has been fixed by making strings use char and not Uint8 (please, people, if it's binary data, use Uint8, if it's text, use char or if necessary unsigned char... not only does it cut down on warnings, but it's more self-documenting if you see a char and you know it means text)

 

I don't doubt it could have been done a bit better, perhaps moving further away from unsigned char and doing the checks that depend on it (like for colours) only occasionally, but as it is it's a lot cleaner and no warnings (at least for me)

 

I'll put it in with several commits in a couple of minutes

Share this post


Link to post
Share on other sites

okay, finished going through it, at least for my set of defines (and no, I don't care to do it for every combination ;) )

 

quite a few times, a daunting few pages of warnings has been fixed by making strings use char and not Uint8 (please, people, if it's binary data, use Uint8, if it's text, use char or if necessary unsigned char... not only does it cut down on warnings, but it's more self-documenting if you see a char and you know it means text)

 

I don't doubt it could have been done a bit better, perhaps moving further away from unsigned char and doing the checks that depend on it (like for colours) only occasionally, but as it is it's a lot cleaner and no warnings (at least for me)

 

I'll put it in with several commits in a couple of minutes

Entropy has prefferred to use Uint8 instead of char in many places, part of the reason is that the color codes turn negative if it's a char.

Share this post


Link to post
Share on other sites

okay, committed now... appologies to anyone who was (legitimately) editing and had to deal with some conflicts... and lets try to keep the number of warnings down, kay? ;)

Share this post


Link to post
Share on other sites

okay, committed now... appologies to anyone who was (legitimately) editing and had to deal with some conflicts... and lets try to keep the number of warnings down, kay? ;)

Wow, nice work, it's nearly a clean compile on gcc version 4.1.2. I've been using version 3.4 because of the warning overload. Here are the remaining warnings I get from a clean CVS now:

cache.c: In function ‘cache_dump_sizes’:
cache.c:66: warning: pointer targets in passing argument 1 of ‘safe_snprintf’ differ in signedness
cache.c:71: warning: pointer targets in passing argument 1 of ‘strlen’ differ in signedness


new_actors.c: In function ‘custom_path’:
new_actors.c:245: warning: pointer targets in passing argument 1 of ‘safe_snprintf’ differ in signedness
new_actors.c:246: warning: pointer targets in passing argument 1 of ‘gzfile_exists’ differ in signedness
new_actors.c:247: warning: pointer targets in passing argument 2 of ‘my_strcp’ differ in signedness
new_actors.c:252: warning: pointer targets in passing argument 1 of ‘safe_snprintf’ differ in signedness
new_actors.c:253: warning: pointer targets in passing argument 1 of ‘gzfile_exists’ differ in signedness
new_actors.c:254: warning: pointer targets in passing argument 2 of ‘my_strcp’ differ in signedness
new_actors.c: In function ‘actor_wear_item’:
new_actors.c:277: warning: pointer targets in passing argument 1 of ‘safe_snprintf’ differ in signedness
new_actors.c:290: warning: pointer targets in passing argument 1 of ‘my_tolower’ differ in signedness
new_actors.c:291: warning: pointer targets in passing argument 1 of ‘safe_snprintf’ differ in signedness
new_actors.c:299: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:299: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:300: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:300: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:305: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:305: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:372: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:372: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:386: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:386: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:399: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:399: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:415: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:415: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:416: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:416: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:417: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:417: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:418: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:418: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:435: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:435: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:436: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:436: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:454: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:454: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:455: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:455: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c: In function ‘add_enhanced_actor_from_server’:
new_actors.c:655: warning: pointer targets in passing argument 1 of ‘my_strncp’ differ in signedness
new_actors.c:677: warning: pointer targets in passing argument 1 of ‘my_tolower’ differ in signedness
new_actors.c:678: warning: pointer targets in passing argument 1 of ‘my_tolower’ differ in signedness
new_actors.c:681: warning: pointer targets in passing argument 1 of ‘strlen’ differ in signedness
new_actors.c:698: warning: pointer targets in passing argument 1 of ‘safe_snprintf’ differ in signedness
new_actors.c:699: warning: pointer targets in passing argument 1 of ‘safe_snprintf’ differ in signedness
new_actors.c:738: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:738: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:739: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:739: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:740: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:740: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:741: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:741: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:743: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:743: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:744: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:744: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:745: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:745: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:746: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:746: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:747: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:747: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:748: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:748: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:750: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:750: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:752: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:752: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:753: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:753: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:755: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:755: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:756: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:756: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:764: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:764: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:776: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:776: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:786: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:786: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:792: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:792: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness
new_actors.c:801: warning: pointer targets in passing argument 2 of ‘custom_path’ differ in signedness
new_actors.c:801: warning: pointer targets in passing argument 3 of ‘custom_path’ differ in signedness

minimap.c: In function ‘display_minimap_handler’:
minimap.c:61: warning: pointer targets in passing argument 3 of ‘draw_string_small’ differ in signedness

Share this post


Link to post
Share on other sites

well, the first I just fixed in CVS... the second hunk is -DCUSTOM_LOOK, third is -DMINIMAP (neither of which my clean tree had set)

 

I guess I can add some more defines and do more of this... it's good for my toadstool count, if nothing else

Share this post


Link to post
Share on other sites

okay, committed now... appologies to anyone who was (legitimately) editing and had to deal with some conflicts... and lets try to keep the number of warnings down, kay? ;)

And you didn't check it into a branch either! We are in a feature freeze and cleanups like this while we are trying to stabilize the code for a release are not a good idea because new bugs can creep in!

 

Everyone, please spend several days now checking to make sure there aren't any unexpected side effects,

Share this post


Link to post
Share on other sites

I didn't do a blind search'n'replace, I looked at the changes... nearly all of them were people using a Uint8 for a string... and then using it as if it was a string, or cases where libxml stuff wanted an xmlChar (or the other way around)

in these cases, I did in code what the compiler automatically does, casting it (at a slightly higher level when the variable declaration was wrong but compatible)... the only difference is that the compiler won't complain

the few changes that weren't just compiler warnings were genuine... a 64-bit issue, cleanup a potential hole, stuff like that (and also simple enough that I don't see a need for them to wait)

it's possible, although I think it quite unlikely, that my changes could cause additional bugs (and hence why I split it into several commits)... and I think reducing the number of warnings will make it easier to track other bugs... if people can run with -Wall normally, then when something does pop up it'll be seen... until now, if you had -Wall, you would have a hard time finding the error message you want

Share this post


Link to post
Share on other sites

well, the first I just fixed in CVS... the second hunk is -DCUSTOM_LOOK, third is -DMINIMAP (neither of which my clean tree had set)

I had not set those myself either. I just checked by deleting make.conf and also by a clean download from CVS and both are enabled by default. That's on Linux at least.

Share this post


Link to post
Share on other sites

*shrug* my tree had them off, hence why I didn't get the warnings... anyway, that new_actors stuff is great to illustrate my point about people using Uint8 when they should use char... all those warnings above were solved thus:

Index: new_actors.c
===================================================================
RCS file: /cvsroot/elc/elc/new_actors.c,v
retrieving revision 1.105
diff -r1.105 new_actors.c
239c239
<	   unsigned char buffer[256];
---
>	   char buffer[256];
267c267
<	   unsigned char playerpath[256], guildpath[256], onlyname[32]={0};
---
>	   char playerpath[256], guildpath[256], onlyname[32]={0};
493,494c493,494
<	   unsigned char playerpath[256], guildpath[256];
<	   unsigned char onlyname[32]={0};
---
>	   char playerpath[256], guildpath[256];
>	   char onlyname[32]={0};
651c651
<			   unsigned char buffer[256], *name, *guild;
---
>			   char buffer[256], *name, *guild;
660c660
<			   for(name=buffer; *name && (*name >= 127+c_lbound) && (*name <= 127+c_ubound); name++);
---
>			   for(name=buffer; *name && IS_COLOR((unsigned char)*name); name++);
668c668
<			   for (guild = name; *guild && ((*guild < 127 + c_lbound) || (*guild > 127 + c_ubound)); guild++);
---
>			   for (guild = name; *guild && IS_PRINT((unsigned char)*guild); guild++);

Share this post


Link to post
Share on other sites

I just got the CVS update to work again and noticed many files got updated, so I guess thats some progress and work done, but...

1>elc - 0 error(s), 2252 warning(s)

is still what I get, with level 3 warnings accepted. Sure there are probably about 1000 warnings on double to float conversions and 1000 on Cal but well, isn't it possible to reduce some of these alittle? Might solve some indirect future problem, or at least clean it up alittle so it's easier to spot bugs later on.

Edited by Beaverhunter

Share this post


Link to post
Share on other sites

don't use MSVC warnings 3 or 4, it's way over the top. 1 or maybe 2 will do.

 

I have reduced the bugs. a lot. if you don't go overboard with the warnings, it's quite sparse now

Share this post


Link to post
Share on other sites

don't use MSVC warnings 3 or 4, it's way over the top. 1 or maybe 2 will do.

 

I have reduced the bugs. a lot. if you don't go overboard with the warnings, it's quite sparse now

 

Hmm oki, well as I mentioned most were of the same type and very non-harmful, so yes your right.

But isn't it possible to fix those double to float warnings? Or all the Cal 'class' warnings and functions?

 

Level 3:

1>elc - 0 error(s), 2252 warning(s)

 

Level 2:

1>elc - 0 error(s), 1231 warning(s)

 

Level 1:

1>elc - 0 error(s), 438 warning(s)

 

Level 0:

1>elc - 0 error(s), 0 warning(s) (sometimes 3-4 warnings though)

Edited by Beaverhunter

Share this post


Link to post
Share on other sites

yes, double to float could be fixed. but it won't be. if you want to know why it's pointless, ask google

going by that, I'd only be interested in getting the level one warnings cleared up

Share this post


Link to post
Share on other sites

yes, double to float could be fixed. but it won't be. if you want to know why it's pointless, ask google

going by that, I'd only be interested in getting the level one warnings cleared up

 

Oki, true it's pointless but well every program should try to get 0 warnings in the end, that should be ideal anyway. But well lets go for the level 1 warnings then. Here is a sample of what most of them consist of:

 

1>c:\dev\elc\cal.h(15) : warning C4273: 'CalCoreMesh_Scale' : inconsistent dll linkage

1> c:\dev\elc\cal3d_wrapper.h(268) : see previous definition of 'CalCoreMesh_Scale'

1>.\stats.c(387) : warning C4113: 'Sint16 (__cdecl *)()' differs in parameter lists from 'Sint16 (__cdecl *)(void)'

Share this post


Link to post
Share on other sites
Oki, true it's pointless but well every program should try to get 0 warnings in the end, that should be ideal anyway.
if you want to say things like that, PM yourself...
1>c:\dev\elc\cal.h(15) : warning C4273: 'CalCoreMesh_Scale' : inconsistent dll linkage
1>		c:\dev\elc\cal3d_wrapper.h(268) : see previous definition of 'CalCoreMesh_Scale'
1>.\stats.c(387) : warning C4113: 'Sint16 (__cdecl *)()' differs in parameter lists from 'Sint16 (__cdecl *)(void)'

guess what... these are MSVC issues... don't like them? fix them, or get someone who uses MSVC to fix them

 

if you want no warnings emitted, then disable warnings. you shouldn't be editing the code anyway, and the warnings are mostly for people who write code

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

  • Recently Browsing   0 members

    No registered users viewing this page.

×