11 Sep, 2008, Nash wrote in the 1st comment:
Votes: 0
The function
int find_random_exit (ROOM_INDEX_DATA *troom)

crashes here
if(troom->exit[i])

because troom apparently doesn't exist

this is the call for the function
if( (door = find_random_exit(ch->in_room)) == -1)

this is the data at that point in code
(gdb) p ch->in_room
$2 = (ROOM_INDEX_DATA *) 0x2b5b111a09a8

and this is the data where it crashes in find_random_exit
(gdb) p troom
$1 = (ROOM_INDEX_DATA *) 0x111a09a8


in the call, in_room works, I can read the vnum, room name etc, but by the time it reaches find_random_exit it's…… gone?
Any idea what I am doing to violate my pointers?
11 Sep, 2008, David Haley wrote in the 2nd comment:
Votes: 0
Without more context, i.e. the lines of code in between the call where everything is fine and the line where it's no longer fine, it's very hard to tell…

Are you sure that 'i' is a valid subscript for the 'exits' array? Are you sure that the exits array is valid?
11 Sep, 2008, quixadhal wrote in the 3rd comment:
Votes: 0
Nash said:
The function
int find_random_exit (ROOM_INDEX_DATA *troom)

crashes here
if(troom->exit[i])

because troom apparently doesn't exist


Check to make sure that troom is a valid pointer to an actual ROOM_INDEX_DATA structure. Just being non-NULL isn't enough, it could be pointing to a random spot in memory (if the pointer were uninitialized).

Also, make sure that troom->exit is a valid pointer to an array of exit structures, and finally that i is set to a value that isn't out of bounds (either negative, or greater than the array size).

Nash said:
this is the call for the function
if( (door = find_random_exit(ch->in_room)) == -1)

this is the data at that point in code
(gdb) p ch->in_room
$2 = (ROOM_INDEX_DATA *) 0x2b5b111a09a8


and this is the data where it crashes in find_random_exit
(gdb) p troom
$1 = (ROOM_INDEX_DATA *) 0x111a09a8


Double check that ch points to a valid character structure. It probably does, but it IS possible for it to point to leftover memory that you freed and that got reallocated for some other kind of things. It won't seg fault becaue it's still memory owned by your process, but it won't be the right data either.

If ch->inroom looks like it is a valid ROOM_INDEX_DATA structure, then something between the entry point of the function and the point it crashes is changing either your local variable, or something in the exits[] array, or you're running off the end of the array.

Since the two pointers point to different chunks of memory, it might be good to set a trap to halt your code when the value of troom changes (I don't remember how… it's been a few years since I've really used gdb).
11 Sep, 2008, Nash wrote in the 4th comment:
Votes: 0
int find_random_exit(ROOM_INDEX_DATA *room){
int doors[6],count = -1;
for(int i = 0;i < 6;i++)
if(room->exit[i])
doors[(++count) - 1] = i;
return count == -1 ? count : doors[number_range(0,count)];
}


I pass ch->in_room to that function, and while in the block of code that calls find_random_exit ch->in_room->name outputs the name of the room, but when I move down the stack to the crash point, room->name is nada. In the ch's structure, it's ROOM_INDEX_DATA *in_room.

Davion: Edited for some code tagging. Silly Nash.
11 Sep, 2008, David Haley wrote in the 5th comment:
Votes: 0
Why does that code say "room", but the gdb output was talking about "troom"? Are we talking about the exact same function or have you changed it?

Anyhow, the problem is that you're accessing doors[-1] on the first time you assign into the doors array: ++(-1) - 1 = -1. That will be stomping all over your stack, causing the variables to get bogus values. Instead, why not set count to 0, and set doors[count++] to i?
11 Sep, 2008, Sandi wrote in the 6th comment:
Votes: 0
Well… there's no point in cycling through 6 is because if(room->exit) will always be either true or false.
And since the door numbers are ordinal numbers, you can't find one with number_range.


This is from Merc:
/*
* Generate a random door.
*/
int number_door( void )
{
int door;

while ( ( door = number_mm() & (8-1) ) > 5)
;

return door;
}


From the flee code:
door = number_door( );
if ( ( pexit = was_in->exit[door] ) == 0
|| pexit->to_room == NULL
|| IS_SET(pexit->exit_info, EX_CLOSED)
|| (IS_NPC(ch)
&& ( IS_SET(pexit->to_room->room_flags, ROOM_NO_MOB)
|| (IS_SET(ch->act, ACT_STAY_AREA)
&& pexit->to_room->area != ch->in_room->area) )) )
continue;


Hope that helps.
11 Sep, 2008, David Haley wrote in the 7th comment:
Votes: 0
Sandi said:
Well… there's no point in cycling through 6 is because if(room->exit) will always be either true or false.

It's actually room->exit– but the forum is treating that as an italics code.

FWIW I think it's better to encapsulate the obtaining of a valid random door in a single function, instead of having a function that returns a random number in the range of possible (as opposed to actual) doors.
11 Sep, 2008, Sandi wrote in the 8th comment:
Votes: 0
DavidHaley said:
Sandi said:
Well… there's no point in cycling through 6 is because if(room->exit) will always be either true or false.

It's actually room->exit– but the forum is treating that as an italics code.
Oops! *insert bonk self on head smiley here *


DavidHaley said:
FWIW I think it's better to encapsulate the obtaining of a valid random door in a single function, instead of having a function that returns a random number in the range of possible (as opposed to actual) doors.
These days, you're right. My post was slightly tongue-in-cheek, if you put a suspenseful pause between the random number generation and checking for validity.
11 Sep, 2008, David Haley wrote in the 9th comment:
Votes: 0
Oh, I see that now – earlier I was in "expedient" mode and didn't catch the irony. :redface:
11 Sep, 2008, quixadhal wrote in the 10th comment:
Votes: 0
Nash said:
int find_random_exit(ROOM_INDEX_DATA *room){
int doors[6],count = -1;
for(int i = 0;i < 6;i++)
if(room->exit[i])
doors[(++count) - 1] = i;
return count == -1 ? count : doors[number_range(0,count)];
}


#define NUM_DOORS 6
int find_random_exit(ROOM_INDEX_DATA *room)
{
int doors[NUM_DOORS];
int i = 0;
int count = 0;

if( !room || !room->exit )
return -1;

for( i = 0; i < NUM_DOORS; i++)
{
doors[ i ] = -1;
if( room->exit[ i ] )
doors[ count++ ] = i;
}
return count ? doors[ number_range( 0, count - 1) ] : -1;
}


Oh, and that merc code will break if you have diagonal directions, since it assumes the number of valid directions will be 7 or less. Written back in the days when someone thought 3 bits was better than 8, yet used an integer array anyways. *shakes head*
11 Sep, 2008, David Haley wrote in the 11th comment:
Votes: 0
The constant should already be defined somewhere else, right? It probably shouldn't be redefined at the function…

quixadhal said:
doors[ i ] = -1;

Why do this? You don't touch the array beyond "count", so might as well leave it as-is. Not that it really matters much in terms of efficiency, but…
12 Sep, 2008, Conner wrote in the 12th comment:
Votes: 0
Sandi said:
Oops! *insert bonk self on head smiley here *

Here ya' go!
12 Sep, 2008, quixadhal wrote in the 13th comment:
Votes: 0
DavidHaley said:
The constant should already be defined somewhere else, right? It probably shouldn't be redefined at the function…

quixadhal said:
doors[ i ] = -1;

Why do this? You don't touch the array beyond "count", so might as well leave it as-is. Not that it really matters much in terms of efficiency, but…


Well, the original version was hard coded for the number 6. I have no idea what the constant for number of directions might be on the MUD in question, so I put that in as a subtle hint that hard coding numbers is bad, and #defines are good.

Actually, I do assign the array… all the elements will be set to -1, but the ones that are < count get set to a direction number. The loop does go all the way through.

As to why, I originally though it would pick a door at random and return -1 for non-existing ones, rather than returning one from the set of doors that exist. You could probably remove that line. :)
12 Sep, 2008, David Haley wrote in the 14th comment:
Votes: 0
Sorry, I meant that while you assign to the whole array, you only read up until count, which is guaranteed to be something you assigned a door number to.
0.0/14