/* command for retrieving stats */ -int get_curr_stat( const CHAR_DATA *ch, int stat ) +int get_curr_stat( const CHAR_DATA *ch, int _stat ) { int max = 0;
@@ -875,9 +875,9 @@
else { - max = pc_race_table[ch->race].max_stats[stat] + 4; + max = pc_race_table[ch->race].max_stats[_stat] + 4;
- if ( class_table[ch->iclass].attr_prime == stat ) + if ( class_table[ch->iclass].attr_prime == _stat ) max += 2;
if ( ch->race == race_lookup( "human" ) ) @@ -886,19 +886,19 @@ max = UMIN( max, 25 ); }
- return URANGE( 3, ch->perm_stat[stat] + ch->mod_stat[stat], max ); + return URANGE( 3, ch->perm_stat[_stat] + ch->mod_stat[_stat], max ); }
/* command for returning max training score */ -int get_max_train( CHAR_DATA *ch, int stat ) +int get_max_train( CHAR_DATA *ch, int _stat ) { int max = 0;
Yeah, I'm kindof curious where cygwin includes stat.h, as linux apparently doesn't put it in the same place. The makefile patch will probably make things cleaner. I hadn't put in a rule since the error isn't fatal under GNU make, but I did dislike seeing the error message.
The error is caused by the "-Werror -Wshadow" combination. Because it's not an error at all, as it's perfectly legal and proper C to declare local variables that shadow globals. Also -Wparentheses cause perfectly legal and intelligible C to be flagged as errors.
Yeah, I'm kindof curious where cygwin includes stat.h, as linux apparently doesn't put it in the same place.
It's a newlib v. glibc difference. You might encounter endless variances in the global namespace on many different unices and versions. So -Wshadow is platform and even version dependent.
I do know it's perfectly legal to shadow definitions in local namespaces, but I've found that the vast majority of the time, it's not done because of someone trying to reuse code, but rather someone doing a cut-and-paste job, or not realizing that they just covered up something global.
In big projects that pass through dozens of hands over the years, it usually causes more harm than good, as someone sees a global variable "buf", and 12 pages down does a strcpy into it and then 7 pages further down, can't figure out why buf has the "old" value. That's actually the main (and really only) reason I don't like declaring variables inside scope brackets vs. the old school way of having to put them at the top of the function.
Using -Werror is certainly something that has caused much debate, but I stand by it, since it's a very rare construct that can't be worked around, and it forces people to deal with little errors while they're still little, rather than ignoring the warnings until there are so many they don't have the fortitude to sit down and tackle them all.
And, if somebody does just need their code to compile, and really intends to deal with the warnings later (or give me the finger and ignore them forever!), it's easy to edit that flag out of the Makefile. :)
No -Werror is fine. The issue is -Wshadow and -Wparentheses flag perfectly good C code. The problem is the patch likely just reintroduces the same warning (in this case error) into those trying to compile with MinGW or DevC++ as _stat is probably defined by their LIBC headers (haven't tested it). In the case of -Wshadow you insist that the local names never conflict with global names. You make the code unportable and subject to the whimsy of global namespace pollution of different versions of gcc, libc, third party libraries and different unix flavors. They are advisory warnings.
quixadhal said:
I do know it's perfectly legal to shadow definitions in local namespaces, but I've found that the vast majority of the time, it's not done because of someone trying to reuse code, but rather someone doing a cut-and-paste job, or not realizing that they just covered up something global.
Well this case in point, declaring a local variable called 'stat' is bad?
Let me give you contrived examples.. suppose I wanted to include a third party library into my project. And it's header declares functions named i(), in(), name(), out(), etc. Then I'd have to change every local variables by the same name in all my code. That's why I think it's a silly sort of warning.
IMO, it's bad because it shares the same name as the stat() function call. It also looks ugly to see things like ch->mod_stat[stat] = 0. I renamed it to stat_index, since that's how it's used.
Then again, I was perfectly happy with dprintf(), back in 1995. Now, it's a glibc call, so I had to rename it to desc_printf()… that being a function to print to descriptors.
This is why I mentioned that the variable name _stat was not ideal in my post.
Tricky
Any variable name the function itself does not otherwise use ought to be fine. If you want to declare is as int arctan that ought to be good. It's -Wshadow that's stupid, IMHO. ;-)
Then again, I was perfectly happy with dprintf(), back in 1995. Now, it's a glibc call, so I had to rename it to desc_printf()… that being a function to print to descriptors.
Those errors aren't flagged by -Wshadow. -Wshadow flags local variables as invalid. C isn't COBOL. It's got scope.
I solved this with the following patch…
The Makefile patch was just to make sure dep was automatically made.
Obviously using _stat isn't ideal, however I am sure you can find a proper variable name for it.
Tricky
Edit: Forgot to mention it works/runs with a few unknown skills warnings in the log file. Also changed rom to ram in the startup script in ./Ice/area