Don’t write functions to return local arrays

Consider the following function (taken from Forgotten Times II):

char* CClassicRPGDoc::levelLocationID(int which_level)
{

char center_location_ID[LABELLENGTH];
int center_location_index = current_location;

char retval[LABELLENGTH];

strcpy(center_location_ID, location_IDs[current_location]);

int counter;
if(which_level > 0)
{
for(counter = 1; counter <= which_level; counter++)
{
strcpy(center_location_ID, locations[center_location_index].getTop().getLocationID());
center_location_index = getLocationIndex(center_location_ID);
}
}
if(which_level = which_level; counter–)
{
strcpy(center_location_ID, locations[center_location_index].getBase().getLocationID());
center_location_index = getLocationIndex(center_location_ID);
}
}
if(which_level != 0)
{
ASSERT(true);
}
strcpy(retval, center_location_ID);
return retval;
};

The above function is used in the display of multi-level buildings and was causing me no end of grief. For some reason, instead of returning what I expected (i.e., the ID of the specified floor of a building) it was consistently returning the ID of the floor (i.e., the one the character was in). Even worse, its behavior changed when I ran it through the debugger, and when I traced the values of the variables all the way to the end of the function, they looked the way they were supposed to.

The problem, of course, was the statement in bold text. While the retval[] string had the right value all the way to the end of the function, once the function exited, the memory that string had occupied was deallocated. As a result, rather than passing back the string I intended, I was passing back a memory location which, by that time, was being used for something else (apparently for storing the ID of the current floor, though I have no idea why). Very annoying.

While there are a number of ways to solve the problem (including reworking the logic so that the problem didn’t come up in the first place), I chose the one that was easiest for me, and made retval[] a static variable. Not the most artful way of doing it, but, since static variables are maintained between function calls, it did the job. The final results are that the last upload of Forgotten Times II has a reasonably functional implementation of multi-story building display, and that I’ve learned to avoid writing functions to return local arrays.

Advertisements

Tags: , ,

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s


%d bloggers like this: