SIGSEGV crashing program

[dawtsf1187]

Hello. Let me start by warning you that my program is very large and it may require a bit of patience on your part to truly understand the problem I'm having with my code. I've tried everything I can think of to fix this, and so now I humbly request some help troubleshooting the problem.

The root of the issue is that I receive a Segmentation Fault error when one particular function is called. I have gotten this error before, usually when I stupidly attempt to write a value to an array that doesn't exist. My function is called itemaction, and it contains an int called xcounter. I initialize xcounter to 0, but for some reason, within the function is gets changed to a ridiculous number (5124883 in my last attempt). I use xcounter to loop through an array (inventoryname[100]) that has 100 elements, so I assume the error is thrown when the program tries to access the non-existent inventoryname[5124883].

I set a watch on inventoryname[xcounter], and receive this text when the error occurs:

inventoryname[xcounter] = The program being debugged was signaled while in a function called from GDB. GDB has restored the context to what it was before the call. To change this behavior use "setunwindonsignal off"

Evaluation of the expression containing the function (std::string::c_str() const) will be abandoned.

Program received signal SIGSEGV
Segmentation fault

So, I suppose some code is needed. I'll try to include the relevant pieces, and if necessary I can upload the entire thing to a pastebin or something, but I don't want to bother you all by giving you 6,000 lines to work through.

I define these variables globally:

string inventoryname[100], inventorycat[100];
int inventorynum[100], removednum;
string packname = "DEFAULT";
double maxinvweight, invweight; 

The program is a game, and throughout its execution, you fill your inventory with items (which go into the respective global arrays). I have made a function called itemaction which allows you to either display the stats for an item or remove the item from your inventory. Code below:

string itemaction (string itemname, string action, int tempcounter){         //Accepts stringname of an item, matches it to a class, and performs an action like EQUIP, UNEQUIP, DISPLAY STATS, DISCARD, etc., and returns itemnumber removed

int xcounter = tempcounter;
item newitem;                  //create an empty class item

//Here, I create all of the available items which I'm omitting for space's sake

if(itemname==leatherboots.stringname) newitem = leatherboots;  //Then I have a line of code like this for each item so I can determine the item that the user chose

if (action=="DISPLAY"){
xcounter = 0;
newitem.display_stats(1,newitem.itemcat);
}//end of DISPLAY if

else if (action=="REMOVE"){
xcounter = 0;
//cout<<"MADE IT HERE"<<endl;

for(xcounter=0;inventorynum[xcounter]!=0;xcounter++){              //Loop through inv starting at the beginning and going until an empty spot is reached (meaning inv must be compressed)
if(itemname==inventoryname[xcounter]){                           //If the item they selected matches the item in the inv, remove it
inventoryname[xcounter]="X";
inventorynum[xcounter]=0;
inventorycat[xcounter]="X";
cout<<"You remove the "<<itemname<<" from your pack. ";
invweight = invweight - newitem.weight;                  //deduct the weight from your pack
while(getchar() != '\n');
wipescreen();
}

}//end of for

compressinv();

removednum = newitem.itemnumber;                          //set global var to itemnumber

}//end of REMOVE


else cout<<"ERROR: Incorrect action coded";

}

This function is called after filling in string variable selecteditem with the name of an item from a list. Here is the code where the function is called:

if(selecteditem=="DONE"){
     choiceok == 1;     //If they chose DONE, continue with the loop
     wipescreen();
}
else {
wipescreen();
itemaction(selecteditem,"DISPLAY",0);     //Otherwise, they chose an item, so display the stats
cout<<"REMOVE the "<<selecteditem<<"? Or are you DONE?"<<endl<<">";
gets(choice2);
UP(choice2);

if(strcmp(choice2,REMOVE)==0) {                 //If they choose to remove the item
    fireloop++;                                 //Time is spent
    itemaction(selecteditem,"REMOVE",0);          //Remove it from the pack; function sets removednum to item number that they removed
    for(counter=0;counter<11;counter++){
    if(removednum==bedroomitem[counter].itemnumber) bedroomitem[counter].stringname = selecteditem;  //if the removed item is one of the items of clothing in the bedroom, replace it on the list
    }
}

I know this may be difficult to help with given only these bits of code. If you have any questions about variables or program order or anything, just ask and I'll try to give more detail. I'm teaching myself how to code c++, so I realize that some of my syntax may not be perfect.

If it helps, I'm using Code::Blocks 10.05 with the GNU GDB 6.8 debugger. I'm running on a Windows 7 x64 PC.

Thanks for your help, and sorry for the long and complicated post!

[ToniAz]

If I understand your for loop correctly, you're iterating over all items in your inventory until you find an empty spot, which you use a value of 0 to denote that it's empty.

The loop structure is ill:

for(xcounter=0;inventorynum[xcounter]!=0;xcounter++)

what if inventorynum[xcounter] is never 0? When does the loop stop? It keeps going and xcounter is incremented. You're better use a while loop, where the condition is (finding an element with zero value or reaching the end of the list).

Where did you initialize your inventory list? You just declared it so the chances are that its elements are weird looking numbers, strings, or whatever. Plus, why do you use inventorynum?

This might not be what you, but could you explain to me what you mean by "(meaning inv must be compressed)"?


I haven't look through the rest of the code.

[dawtsf1187]

Thanks for the response! You're correct in guessing what I'm trying to make this function do. At this particular point in the game, you are filling your inventory with items from a numbered list. If you choose an item, it gets added to the three inventory arrays (each item has a name, a number, and a category that gets saved; other stats are not saved off). If you want to remove an item, you have to look at your pack, then choose the item, then specify that you wish to REMOVE it. At this point, the program does the looping you see to determine what item was chosen and then resets the inventory arrays to "X" (for strings) or 0 (for ints). The final step is replacing the removed item on the list that they initially chose from.

You make a good point about the loop potentially never stopping. At this point, it should never reach above 20 or so because the player can't hold that many items. Still, it makes sense to code for that eventuality, so I modified the code to disallow the counter to exceed 99. Good catch. Unfortunately, it didn't prevent the SIGSEGV from crashing the program.

As for your other questions, I initialized the inventory list in a separate function that I didn't originally show. Here it is in part:

void initializeinv(void){

int counter = 0;

for (counter=0;counter<100;counter++){

inventoryname[counter] = "X";
inventorycat[counter] = "X";
inventorynum[counter] = 0;

}

The rest of the function creates all the items of class "item" and assigns stats to them. I have to define them in multiple places because for some reason their scope is restricted to the function in which they are created. But that's another problem that isn't stopping my progress, so it's less important.

Sorry for the omission of inventory compression. What I mean by that is that the inventory can't have gaps in it. All of the items should be continuous at the beginning of the array, not spread out through it. I made another function that does this:

void compressinv (void){            //shuffles through inventory array and removes gaps

string tempname;
string tempcat;
int tempnum = 0, counter = 0, done = 0;

for(counter = 0;done != 1;counter++){

if((inventorynum[counter]==0)&&(inventorynum[counter+1]!=0)){            //if you reach a blank item and the next item is not blank
inventorynum[counter]=inventorynum[counter+1];                          //Swap the blank item with the populated item
inventorynum[counter+1]=0;                                              //Then erase the duplicate
inventoryname[counter]=inventoryname[counter+1];                        //Repeat for name and cat
inventoryname[counter+1]="X";
inventorycat[counter]=inventorycat[counter+1];
inventorycat[counter+1]="X";
}

else if((inventorynum[counter]==0)&&(inventorynum[counter+1]==0)) done = 1;     //if you reach two blank items in a row, you're done
else if (counter == 99) done = 1;                                                //since there can only be 100 items in inv now, end here
else done = 0;

}//end of for

}

Please let me know if there is anything else that needs to be clarified. This SIGSEGV error is making it difficult to debug since breakpoints behave all wonky within itemaction().

[dawtsf1187]

Well, after hours of toil, I discovered the problem. Lo and behold, it was a simple mistake, as it seems it always is. My function itemaction() was declared as type string (since in a previous version of the code, it returned the string name of the item removed), but there was no return statement in the function. When I called the function in code, I was calling it in isolation, not setting a string equal to it. Not sure why this caused a SIGSEGV and not a compiler error, but the problem is solved now.

Changing
string itemaction (string itemname, string action, int tempcounter)

to

void itemaction (string itemname, string action, int tempcounter)

solved the problem.

Thanks for the help (if not in solving the problem, then for making me keep my mind on it)!