20 Jan, 2009, tphegley wrote in the 1st comment:
Votes: 0
I'm on ubuntu 8.10 using g++-4.1 as my makefile. Smaugfuss1.8

When I run the game it crashes shortly after startup. When ran in gdb I get this:

Program received signal SIGSEGV, Segmentation fault.
0xb7d262ae in strcasecmp () from /lib/libc.so.6
(gdb) list
382 } /* cleanup memory */
383
384 #ifdef WIN32
385 int mainthread( int argc, char **argv )
386 #else
387 int main( int argc, char **argv )
388 #endif
389 {
390 struct timeval now_time;
391 bool fCopyOver = FALSE;
(gdb) bt
#0 0xb7d262ae in strcasecmp () from /lib/libc.so.6
#1 0xb7d26281 in strcasecmp () from /lib/libc.so.6
(gdb)
(gdb) print strcasecmp
$1 = {<text variable, no debug info>} 0xb7d26270 <strcasecmp>
(gdb) frame 1
#1 0xb7d26281 in strcasecmp () from /lib/libc.so.6
(gdb) print strcasecmp
$2 = {<text variable, no debug info>} 0xb7d26270 <strcasecmp>
(gdb) frame 0
#0 0xb7d262ae in strcasecmp () from /lib/libc.so.6
(gdb) print strcasecmp
$3 = {<text variable, no debug info>} 0xb7d26270 <strcasecmp>
(gdb) print 0xb7d26270
$4 = 3084018288
(gdb)


I'm not sure exactly what's going on here. Can someone point me in a direction?

Line 382 is the end of the cleanup_memory function.
strcasecmp is mainly used in imc.c and in david haley's skill table additions in magic.c and tables.c. This version of smaugfuss has lua installed so there are a few instances of strcasecmp in lua_scripting.c


**EDIT**
Smaugfuss 1.9 runs cleanly.
20 Jan, 2009, David Haley wrote in the 2nd comment:
Votes: 0
tphegley said:
(gdb) bt
#0 0xb7d262ae in strcasecmp () from /lib/libc.so.6
#1 0xb7d26281 in strcasecmp () from /lib/libc.so.6

This looks an awful lot like stack corruption. There should be more of a stack here – that you only see those two (most likely) means that something wrote all over the stack somewhere.

The first thing I'd do here is run it through valgrind – it should be able to spot the stack corruption as it happens.
20 Jan, 2009, elanthis wrote in the 3rd comment:
Votes: 0
Running in Valgrind is an easy way to help find these kinds of errors. It usually detects the stack smash immediately and prints out a diagnostic for you.
20 Jan, 2009, tphegley wrote in the 4th comment:
Votes: 0
Yea, I ran it in valgrind and it has to do with my spec_mage_defend which has a mage cast defensive spells on themselves when they arent' fighting.

bool spec_mage_defend( CHAR_DATA * ch )
{

char *spell;
int sn;


if( ch->position == POS_FIGHTING)
return FALSE;

for( ;; )
{
int min_level;

switch ( number_bits( 4 ) )
{
case 0:
min_level = 0;
spell = "armor";
break;
case 1:
min_level = 8;
spell = "kindred strength";
break;
case 2:
min_level = 17;
if ( !IS_AFFECTED (ch, AFF_HASTE))
spell = "haste";
break;
default:
min_level = 20;
if ( !IS_AFFECTED (ch, AFF_SHOCKSHIELD))
spell = "shockshield";
break;
}

if( ch->level >= min_level )
break;
}

if( ( sn = skill_lookup( spell ) ) < 0 )
return FALSE;
do_cast(ch, spell);
return TRUE;
}


here is the valgrind output:

==4632== Conditional jump or move depends on uninitialised value(s)
==4632== at 0x82353E1: spec_cleric_defend (special.c:1375)
==4632== by 0x8245F50: mobile_update() (update.c:860)
==4632== by 0x8248D90: update_handler() (update.c:2323)
==4632== by 0x814648A: game_loop() (comm.c:873)
==4632== by 0x8148353: main (comm.c:531)
==4632==
==4632== Conditional jump or move depends on uninitialised value(s)
==4632== at 0x41E129B: strcasecmp (in /lib/libc-2.8.90.so)
==4632== by 0x81B6814: bsearch_skill_exact(char const*, int, int) (magic.c:291)
==4632== by 0x81B70A5: skill_lookup(char const*) (magic.c:161)
==4632== by 0x82354E8: spec_mage_defend (special.c:1227)
==4632== by 0x8245F32: mobile_update() (update.c:850)
==4632== by 0x8248D90: update_handler() (update.c:2323)
==4632== by 0x814648A: game_loop() (comm.c:873)
==4632== by 0x8148353: main (comm.c:531)
==4632==
==4632== Use of uninitialised value of size 4
==4632== at 0x41E12AE: strcasecmp (in /lib/libc-2.8.90.so)
==4632== by 0x81B6814: bsearch_skill_exact(char const*, int, int) (magic.c:291)
==4632== by 0x81B70A5: skill_lookup(char const*) (magic.c:161)
==4632== by 0x82354E8: spec_mage_defend (special.c:1227)
==4632== by 0x8245F32: mobile_update() (update.c:850)
==4632== by 0x8248D90: update_handler() (update.c:2323)
==4632== by 0x814648A: game_loop() (comm.c:873)
==4632== by 0x8148353: main (comm.c:531)
==4632==
==4632== Invalid read of size 1
==4632== at 0x41E12AE: strcasecmp (in /lib/libc-2.8.90.so)
==4632== by 0x81B6814: bsearch_skill_exact(char const*, int, int) (magic.c:291)
==4632== by 0x81B70A5: skill_lookup(char const*) (magic.c:161)
==4632== by 0x82354E8: spec_mage_defend (special.c:1227)
==4632== by 0x8245F32: mobile_update() (update.c:850)
==4632== by 0x8248D90: update_handler() (update.c:2323)
==4632== by 0x814648A: game_loop() (comm.c:873)
==4632== by 0x8148353: main (comm.c:531)
==4632== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==4632==
==4632== Process terminating with default action of signal 11 (SIGSEGV)
==4632== Access not within mapped region at address 0x0
==4632== at 0x41E12AE: strcasecmp (in /lib/libc-2.8.90.so)
==4632== by 0x81B6814: bsearch_skill_exact(char const*, int, int) (magic.c:291)
==4632== by 0x81B70A5: skill_lookup(char const*) (magic.c:161)
==4632== by 0x82354E8: spec_mage_defend (special.c:1227)
==4632== by 0x8245F32: mobile_update() (update.c:850)
==4632== by 0x8248D90: update_handler() (update.c:2323)
==4632== by 0x814648A: game_loop() (comm.c:873)
==4632== by 0x8148353: main (comm.c:531)
==4632==
==4632== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 39 from 2)
==4632== malloc/free: in use at exit: 2,600,544 bytes in 33,331 blocks.
==4632== malloc/free: 34,820 allocs, 1,489 frees, 2,807,117 bytes allocated.
==4632== For counts of detected errors, rerun with: -v
==4632== searching for pointers to 33,331 not-freed blocks.
==4632== checked 2,975,820 bytes.
==4632==
==4632== LEAK SUMMARY:
==4632== definitely lost: 1,428 bytes in 43 blocks.
==4632== possibly lost: 0 bytes in 0 blocks.
==4632== still reachable: 2,599,116 bytes in 33,288 blocks.
==4632== suppressed: 0 bytes in 0 blocks.
==4632== Rerun with –leak-check=full to see details of leaked memory.
Segmentation fault


This spec is copied from spec_mage. I call it in update.c during mobile_update but I must not have called it correctly.

update.c
switch(ch->Class) {
case CLASS_MAGE:
spec_mage(ch);
spec_mage_defend(ch);
break;
case CLASS_CLERIC:/*
if(ch->Alignment < ALIGNMENT_EVIL)
do_evil_priest_things();
else if(ch->Alignment > ALIGNMENT_GOOD)
do_good_priest_things();
else
do_neutral_priest_things();*/
spec_cleric(ch);
spec_cleric_defend(ch);
break;
default:
spec_warrior(ch);
break;
}


This worked in g++-3.3.6 but I do not have that installed and I am using 4.1. I figure it's the update.c thing that is most likely the problem.
20 Jan, 2009, David Haley wrote in the 5th comment:
Votes: 0
Ah, I think I know what your problem is. Note that in case 2 and the default case, you only set "spell" if some expression is true. You then pass that into skill_lookup – but it's a bogus string.

I'm not sure why this would work with a different version of gcc, though.
20 Jan, 2009, tphegley wrote in the 6th comment:
Votes: 0
So how would I set up an if statement in case 2 and default? I don't want them to cast anything if they are affected by it.

Would it be something like:

case 2:
if ( !IS_AFFECTED (ch, AFF_HASTE))
{
min_level = 17;
spell = "haste";
break;
}
else
break;


case 2:
if ( !IS_AFFECTED (ch, AFF_HASTE))
{
min_level = 17;
spell = "haste";
break;
}
else {
spell = NULL;
break;
}
20 Jan, 2009, Zeno wrote in the 7th comment:
Votes: 0
Before the lookup/cast, do an ifcheck if spell is not bogus/null?
20 Jan, 2009, ghasatta wrote in the 8th comment:
Votes: 0
It's probably good style to initialize your pointer to NULL as well. Then, as Zeno suggested, check for NULL at the end of your loop.

My guess is that this problem was present all along but since it requires certain conditions before it becomes apparent, you haven't noticed it before (or maybe it never came up before).
20 Jan, 2009, David Haley wrote in the 9th comment:
Votes: 0
You could do what Zeno suggested – just make sure you initialize "spell" to NULL first – or you could return from the function entirely after determining that they already have the effect.
20 Jan, 2009, ghasatta wrote in the 10th comment:
Votes: 0
@davidhaley - jinx!
20 Jan, 2009, David Haley wrote in the 11th comment:
Votes: 0
D'oh! I guess I owe you a coke now :tongue:
20 Jan, 2009, tphegley wrote in the 12th comment:
Votes: 0
So something like:

bool spec_mage_defend( CHAR_DATA * ch )
{

char *spell;
int sn;


if( ch->position == POS_FIGHTING)
return FALSE;

spell = NULL;

for( ;; )
{
int min_level;

switch ( number_bits( 4 ) )
{
case 0:
min_level = 0;
spell = "armor";
break;
case 1:
min_level = 8;
spell = "kindred strength";
break;
case 2:
if ( !IS_AFFECTED (ch, AFF_HASTE))
{
min_level = 17;
spell = "haste";
break;
}
else
break;
default:
if ( !IS_AFFECTED (ch, AFF_SHOCKSHIELD)) {
min_level = 20;
spell = "shockshield";
break;
}
else
break;
}

if( ch->level >= min_level )
break;
}

if (spell == NULL)
return FALSE;

if( ( sn = skill_lookup( spell ) ) < 0 )
return FALSE;
do_cast(ch, spell);
return TRUE;
}
20 Jan, 2009, David Haley wrote in the 13th comment:
Votes: 0
Looks good to me, although I don't think you need the breaks – just set if appropriate, and do nothing (leaving the NULL value) if not.
20 Jan, 2009, tphegley wrote in the 14th comment:
Votes: 0
So this would work the same:

case 2:
if ( !IS_AFFECTED (ch, AFF_HASTE))
{
min_level = 17;
spell = "haste";
}
break;
20 Jan, 2009, David Haley wrote in the 15th comment:
Votes: 0
Yes, that's what I would do. (Yeah, I just noticed that my previous statement was unclear: you do need the breaks, I meant that you just don't need to repeat them in both conditions.)
20 Jan, 2009, tphegley wrote in the 16th comment:
Votes: 0
Ok, that's working now.

One last question, the number_bits part in the switch statement. That's going to be a random number between 0-3 right?
20 Jan, 2009, Zeno wrote in the 17th comment:
Votes: 0
Why not just use number_range?
20 Jan, 2009, tphegley wrote in the 18th comment:
Votes: 0
Zeno said:
Why not just use number_range?


number_bits is what it originally used in spec_cast_mage. I was just asking for confirmation that number_bits ( 4 ) would in fact be a random number from 0-3. I guess that it means it is, but just wanted confirmation.

I'm sure number_range would work jsut the same but why would I use it over number_bits? What's the benefit?
20 Jan, 2009, David Haley wrote in the 19th comment:
Votes: 0
Assuming that number_bits(x) generates a number with x bits each of which is randomly 0 or 1:

0000 0
0001 1
0010 2
0011 3
0100 4

1111 15 (2^0 + 2^1 + 2^2 + 2^3 = 1 + 2 + 4 + 8 = 15)

so actually 4 bits is a number from 0 to 15.

As for why you'd use one over the other, there is perhaps an argument to be made for efficiency, but frankly I think that should not matter in the slightest. Trying to be clever about things like this is a great way to make mistakes, and the efficiency gain is completely negligible given the usage pattern.
20 Jan, 2009, Zeno wrote in the 20th comment:
Votes: 0
Why I suggested number_range: readability.
0.0/23