Inconsistant Errors and Crashes

[Zap]

so, the program is meant to run a simulation of food supplies to an island with a variable amount of villages. (if you need the specific details i can provide a pdf)

basically I can have one, both or neither of two problems happen :
1: in the function "pickTargetVillage" the variable nVillages, changes, when it should remain constant. this is most obvious when set to 2, this change causes the program to try and assign a variable to a spot outside the bounds of the array

this can be temporarily fixed by making the variable constant (and passing it as a constant), which brings out the second issue:

2: when the end of the program is reached it tends to crash.

I don't think it has to do with my class KeepRunning, i have used is many times before without a problem. as just using the 'pause' at the end of main results in the same crash. i think this is likely due to the first error.



thanks in advance,
sorry this next bit is quite long.

#include <iostream>
#include <iomanip>
#include <cmath>
#include<time.h>

using namespace std;


//keeps the window open after end of main is reached
class KeepRunning
{
public:
    ~KeepRunning()
    {
         system("PAUSE");         
    }
};


//global constants
const int minFoodPerDrop = 40,  // min food per drop
          maxFoodPerDrop = 60,  // max food per drop
          requiredFoodPerVillage = 100,  // food each village must have
          maxVillages = 10,     // max number of villages
          tableEntries = 18,    // number of table entries
          landedInSea = -1,     // special value used to represent "landed in sea"
          Ntrials=10000;        //number of trials
          
//supplied function(s)
int simulateCampaign   ( int numberOfVillages); 

//my function(s)
int pickTargetVillage  (int numberOfVillages, int unitsReceived[]); //choses village with the lowest recieved units of food
int pickLandingVillage (int numberOfVillages, int targetVillage);   //determines where the drop actually lands
int pickUsableUnits    (void);                                      //determines how many inits survive the fall

int main(void)
{
    
    KeepRunning kr; //runs a destructor which keeps the window open after end of main is reached
    
    srand (time(NULL));  //seed the random number generator
    
    int  numberOfVillages,              //number of vilages on the island as inputr by the user   
         DropData[tableEntries]={ 0 },  //array for storing the number of times the drops required falls in the "bins"
         nDrops=0,  //number of drops required to relive the vilages
         BinSize,   //with of the bins (range per bin in output)
         MinDrops,  //minimum drops required to relieve the island
         pos=0,     //used to define the memory spot in an array
         bin;       //used for output
    
    cout<<"how many vilages on the island? (up to 10) : ";
    cin>>numberOfVillages;
    //"bulletproofing" the input
    while(cin.fail() || numberOfVillages <=0 || numberOfVillages >10)
    {
        cout<<"error: invalid values\n";
        cin.clear();
        cin.ignore(INT_MAX, '\n');  
        cout<<"how many vilages on the island? : ";
        cin>>numberOfVillages;                      
    }
    
    
    
    //as defined in the assignment
    BinSize = 1 + (numberOfVillages / 3);
    MinDrops = numberOfVillages *((requiredFoodPerVillage + (maxFoodPerDrop - 1)) / maxFoodPerDrop);
    
    
    //runs the simulation
    for (int d=0; d<Ntrials; d++)
    {
        nDrops = simulateCampaign(numberOfVillages);
        int i = MinDrops;
        pos=0;
        
        //determines which bin the nDrops belongs to.
        while (nDrops>i)
        {
            i += (BinSize);
            pos++;
        }
        DropData[pos-1]++;
    }
    
    
    //print out the data
    bin = MinDrops;
    cout<<endl<<setiosflags(ios::fixed|ios::showpoint)<<setprecision(3)
        <<"   Number of Drops    Probability (%)  \n"
        <<"---------------------------------------\n";
    for(int i = 0 ; i<tableEntries-1 ; i++)
    {
          cout<<setw(8)<<bin<<" to "<<setw(3)<<bin+BinSize-1<<setw(16)<<(DropData[i]/(Ntrials*1.0))*100<<endl;
          bin += BinSize;
    }
    cout<<setw(8)<<bin<<" and above "<<setw(12)<<(DropData[tableEntries-1]/(Ntrials*1.0))*100<<endl;
}


// Simulates a single relief campaign and returns the number of drops
// required to supply all of the villages.
int simulateCampaign (int numberOfVillages)
{
  int unitsReceived[maxVillages] = { 0 },
      drops = 0,
      targetVillage, 
      landingVillage;
                                              
  
  for (;;)                                               
  {
    // Decide where the next drop should be targeted.
    targetVillage = pickTargetVillage (numberOfVillages, unitsReceived);
    //cout<<targetVillage<<endl;
    if (unitsReceived[targetVillage] >= requiredFoodPerVillage)
    {
      // The village with the least amount of food has enough.
      // The campaign is complete.  Return the number of drops required.
      return drops;
    }

    // Work out where the drop will acually land.
    landingVillage = pickLandingVillage (numberOfVillages, targetVillage);

    if (landingVillage != landedInSea)
    {
      // The drop did not go into the sea.
      // Update the food received by the village where it landed.
      unitsReceived[landingVillage] += pickUsableUnits();
    }
    drops++;
  }
  
}


int pickTargetVillage  ( int numberOfVillages, int unitsReceived[])
{
    int Lowest = unitsReceived[0],
        village=0;
        
    for(int i =0; i < numberOfVillages ; i++)
    {
          if(Lowest > unitsReceived[i])
          {
                Lowest = unitsReceived[i];   
                village=i;
          }     
    }
    //cout<<numberOfVillages<<endl;
    //cout<<village<<endl;
    return village;
}


int pickLandingVillage (int numberOfVillages, int targetVillage)
{
    double x;
    
    x = rand() / (RAND_MAX + 1.0); // x now random: 0 <= x < 1
    if (x < 0.32) 
    {
        return targetVillage;
    } 
    else if 
    (x < 0.32 + 0.12)
    {
         if((targetVillage-2)<0)
         {
              //food is in the water
               return landedInSea;                 
         }
         return targetVillage-2;
    } 
    else if 
    (x < 0.32 + 0.12 + 0.22){
          if((targetVillage-1)<0)
          {
              //food is in the water
               return landedInSea;                 
          }
         return targetVillage-1;
    } 
    else if (x < 0.32 + 0.12 + 0.22 + 0.12) 
    {
         if((targetVillage+2) >= numberOfVillages){
              //food is in the water
               return landedInSea;                 
         }
         return targetVillage+2;
    }else
    {
         if((targetVillage+2)>= numberOfVillages)
         {
              //food is in the water
               return landedInSea;                 
         } 
         return targetVillage+1;
    }
}


int pickUsableUnits(void)
{
     return (rand()%21+40); 
}

i believe i have isolated the first problem to here:

int pickTargetVillage  (const int numberOfVillages, int unitsReceived[])
{
    int Lowest = unitsReceived[0],
        village=0;
    for(int i =0; i < numberOfVillages ; i++)
    {
          if(Lowest > unitsReceived[i])
          {
                Lowest = unitsReceived[i];   
                village=i;
          }     
    }
    //if you uncomment theis you should see it increase steadily
    //cout<<numberOfVillages<<endl;
    return village;
}

[ne555]

    //runs the simulation
    for (int d=0; d<Ntrials; d++)
    {
        nDrops = simulateCampaign(numberOfVillages);
        int i = MinDrops;
        pos=0;
        
        //determines which bin the nDrops belongs to.
        while (nDrops>i)
        {
            i += (BinSize);
            pos++;
        }
        //what if pos has an invalid value?
        DropData[pos-1]++;
    }

If the inner while is never executed, pos remains with 0 and line 84 is out of bounds.
That could modify another variable, causing inconsistencies.

Another things:
_ minFoodPerDrop = 40, // min food per drop That comment is unnecessary. The name of the variable is enough.

_ About your method to keep the console open.
__ The problem is that the destructor could not be called. By instance, when an exception is throwed, a fatal error occurs (like division by zero or dereferencing a NULL pointer), or a call to exit.
The worst part is in those cases is when it would be useful that the program doesn't terminate right away
__ System is evil http://www.cplusplus.com/forum/articles/11153/
__ You better use an IDE or just run the program using the console. (learn cd and ls|dir )

[firedraco]

The problem is that the destructor could not be called. By instance, when an exception is throwed, a fatal error occurs (like division by zero or dereferencing a NULL pointer), or a call to exit.

Actually, the destructor is guaranteed to be called. >_>

The crash is due to stack corruption from writing out of bounds on that array.

[ne555]

http://www.cplusplus.com/forum/beginner/33789/
http://stackoverflow.com/questions/222175/why-destructor-is-not-called-on-exception

If no matching handler is found in a program, the function terminate() (_except.terminate_) is called. Whether or not the stack is unwound before calling terminate() is implementation-defined.

It seems that exit only calls the destructor of global variables. abort doesn't call anything

testing

int main(){
  KeepRunning kr;

  throw 42; //not called

  int *p=NULL;
  *p = 42; //not called

  exit( EXIT_FAILURE ); //not called
}

int main(){
  try{
    KeepRunning kr;
    throw 42;  //called
  }
  catch(...){}
}

[Zap]

ne555 wrote:

 //runs the simulation
    for (int d=0; d<Ntrials; d++)
    {
        nDrops = simulateCampaign(numberOfVillages);
        int i = MinDrops;
        pos=0;
        
        //determines which bin the nDrops belongs to.
        while (nDrops>i)
        {
            i += (BinSize);
            pos++;
        }
        //what if pos has an invalid value?
        DropData[pos-1]++;
    }

If the inner while is never executed, pos remains with 0 and line 84 is out of bounds.
That could modify another variable, causing inconsistencies.

yes. that was definitely the problem. thanks a bunch!

also, i do know that the whole system() stuff is bad, however i am only in first year of university, and that is what they teach/expect us to use. so i can't do much about that.


thanks for your help everyone.