objects declared on the heap causing crash in windows but not linux

[jovispater]

Ok, I've recently started learning C++ based on the "Teach yourself C++ in 21 Days" book, which btw I highly recommend for anyone starting C++ from scratch. Anyway, I've decided to put my newfound skills to work to attempt to replicate an old video game I used to play (purely for practice, not trying to steal someone's game or anything).

Basically, the problem is this: my code compiles fine both in Windows and in Linux. When I run the program, it runs fine in Linux and in Wine. When I run the program on Windows (I've tried three different computers, two running XP and one running Vista), it crashes at certain points. The points where it crashes are at the declaration of ANY type of object on the heap, but this only happens if a Commander object has been created first. For example...

#include <iostream>
#include <Commander.cpp>    // includes Commander.hpp

int main()
{
   using std::cout;  using std::cin;

   int * pInt = new int;    // when running, this code executes fine
   delete pInt;

   Commander me;

   pInt = new int;          // when running, this is where the program crashes

   return 0;
}

I have no idea what the problem could be. Every once in a while it will work on a Windows machine, but 9 times out of 10, it will crash at the declaration of a new object on the heap ONLY IF the Commander has been created. On Linux and Wine, this program runs 100% flawlessly.

Here is a copy of my Commander.hpp file and in my next post I'll paste a copy of my Commander.cpp file. It's a little convoluted I know, I actually started reading my book and handwrote this program on notebook paper while sitting in a bouncing truck driving around Haiti back in March, so I'm aware that some of the things in my code could be a little cleaner. I won't go into details as to why "invenstoryslot" is a pointer to an array of pointers on the heap, but for the time being it was necessary.


Commander.hpp - I've left out code that is too localized to be relevant or that I've been able to comment out and still have the same problem.

class Commander {
      private:
          char name[21];
          int infxp, mobxp, avixp, orgxp;
          int tactics, clout, education, mechapt;
          int freepoints, reincarnations, bank;
          int honor, wins, losses, prestige;

          Unit ** deployslot;
          bool reincSignup;

          int FindDeploySlot();

      public:

          Unit ** inventoryslot;
          Commander();
          ~Commander();

          int FindSlot();               // Find an empty unit slot in inventory
          int Unitcount() const;        // View how many units are currently stored in inventory
          int Deploycount() const;      // View how many units are currently set to deploy
          int UnitSlots() const;        // Calculate maximum inventory unit slots
          int BattleSlots() const;      // Calculate maximum deploying unit slots
          void BuyUnit(UNIT);           // Purchase a unit and add it to inventory
          void SellUnit();              // Sell the unit and clear its slot
          void DeployUnit(int slot);    // Deploy selected unit
          void UndeployUnit(int slot);  // Undeploy the selected unit
};

[jovispater]

Commander.cpp

#include <Commander.hpp>

Commander::Commander(): infxp(0), mobxp(0), avixp(0), orgxp(0),
                        tactics(5), clout(5), education(5), mechapt(5),
                        freepoints(20), reincarnations(0), bank(0),
                        honor(0), wins(0), losses(0), prestige(0),
                        reincSignup(false),
                        tactics(5), clout(5), education(5), mechapt(5)
{
        cout << "In TEST Commander Constructor\n";
        char hero[25];
        cout << "Choose a name for your commander:\t";
        cin >> hero;
        strncpy(name,hero,20);

        inventoryslot = new Unit*[UnitSlots()];
            for (int i = 0; i < UnitSlots(); i++) {
                inventoryslot[i] = 0;
            }

        deployslot = new Unit*[BattleSlots()];
            for (int i = 0; i < UnitSlots(); i++) {
                deployslot[i] = 0;
            }
}

Commander::~Commander() { cout << "In Commander Destructor\n"; delete[] inventoryslot; delete[] deployslot; }

int Commander::FindSlot() {
    cout << "In Commander::FindSlot()\n";
    if (Unitcount() < UnitSlots()) {
        int x;

        for (x = 0; inventoryslot[x] != 0; x++) { cout << "FindSlot():\tSlot " << x << " full!\n"; }
        return x;
    } else { cout << "All full!\n"; }
}

int Commander::FindDeploySlot() {
    cout << "In Commander::FindDeploySlot()\n";
    if (Deploycount() < BattleSlots()) {
        int x;

        for (x = 0; deployslot[x] != 0; x++) { cout << "FindDeploySlot():\tSlot " << x << " full!\n"; }
        return x;
    } else { cout << "All full!\n"; }
}

int Commander::BonusPoints(int lvlbank, int reinc) const {
    cout << "In Commander::BonusPoints()\n";
    int a=1, b=0, c=0, points=0;
         while (c<lvlbank) {
               points = (lvlbank/a)+(b);
               c++;

               if (c/(2*a)==(a+1)) {
                    a++;
                    b+=2;
                }
         }
return (points + (reinc*2));
}

int Commander::Unitcount() const {
    cout << "In Commander::Unitcount()\n";
    int a=0, i=0;
    for (; i < UnitSlots(); i++) { if (inventoryslot[i] != 0) { a++; } }
    return a;
}

int Commander::Deploycount() const {
    cout << "In Commander::Deploycount()\n";
    int a=0, i=0;
    for (; i < BattleSlots(); i++) { if (deployslot[i] != 0) { a++; } }
    return a;
}

int Commander::UnitSlots() const {                               // Calculate maximum number of unit slots in inventory
    return (48 + (reincarnations*2));                            // 48 (default starting number) + 2 bonus slots per reincarnation
}

int Commander::BattleSlots() const {                             // Calculate maximum number of deployed units based on tactics score
    cout << "In Commander::BattleSlots()\n";
    if (tactics < 20) { return 6; }
    if ( (tactics >= 20) && (tactics < 40) ) { return 7; }
    if ( (tactics >= 40) && (tactics < 60) ) { return 8; }
    if ( (tactics >= 60) && (tactics < 80) ) { return 9; }
    if ( (tactics >= 80) && (tactics < 100) ) { return 10; }
    if ( (tactics >= 100) && (tactics < 120) ) { return 11; }
    if (tactics == 120) { return 12; }
}

[jovispater]

Commander.cpp continued...

void Commander::BuyUnit(UNIT choice) {
    cout << "In Commander::BuyUnit()\n";
    if (Unitcount() < UnitSlots()) {
        cout << "BuyUnit():  Searching for open slot.\n";
        int i = FindSlot();
        cout << "Slot found in " << i << ". Entering switch\n";

        switch (choice) {
            case (IMP): { inventoryslot[i] = new Imp; cout << "Created a new Imp!\n"; break; }
            case (GHAST): { inventoryslot[i] = new Ghast; cout << "Created a new Ghast in slot " << i << "!\n"; break; }
            case (PEGASUS): { inventoryslot[i] = new Pegasus; cout << "Created a new Pegasus in slot " << i << "!\n"; break; }
            case (BEHEMOTH): { inventoryslot[i] = new Behemoth; cout << "Created a new Behemoth in slot " << i << "!\n"; break; }
            case (EAGLE): { inventoryslot[i] = new Eagle; cout << "Created a new Eagle in slot " << i << "!\n"; break; }
            case (PHOENIX): { inventoryslot[i] = new Phoenix; cout << "Created a new Phoenix in slot " << i << "!\n"; break; }
            default : { ; }
        }
        cout << "BuyUnit():  Completed switch. Returning...\n";
    } else { cout << "Unit slots are all full! Sell a unit first.\n";
    }
}

void Commander::SellUnit() {
    int i;
    cout << "Enter the unit slot you want to clear:\t";
    cin >> i;
     if ((i >= UnitSlots()) || (i < 0)) { cout << "You don't have that slot!\n";
     } else {
         if (inventoryslot[i] == 0) { cout << "This slot is already empty!\n";
         } else {
             if (inventoryslot[i]->SlotDeployed() != -1) {
                 UndeployUnit( inventoryslot[i]->SlotDeployed() );
             }
             delete inventoryslot[i];
             inventoryslot[i] = 0;
         }
     }
}
void Commander::DeployUnit(int slot) {
     if (inventoryslot[slot]->SlotDeployed() == -1) {
      if (Deploycount() <= BattleSlots()) {
          int i = FindDeploySlot();
          deployslot[i] = inventoryslot[slot];
          inventoryslot[slot]->Deploy(i);
      } else {
          cout << "You can't deploy any more units!\n";
      }
     } else {
          cout << "That unit is already deployed!\n";
     }
}
void Commander::UndeployUnit(int slot) {
     if (deployslot[slot] != 0) {
         deployslot[slot]->Deploy(-1);
         deployslot[slot] = 0;
     }
}

...any suggestions?

[jovispater]

Hah! I figured it out. I can't believe I spent countless hours last night trying to figure this out.
The problem was a simple mixup with this piece of code in the Commander constructor:

 deployslot = new Unit*[BattleSlots()];

            for (int i = 0; i < UnitSlots(); i++) {                  // UnitSlots() should be replaced with BattleSlots()

                deployslot[i] = 0;

UnitSlots is used for determining the size of the inventoryslot array, which represents the number of units the commander possesses based on certain members in Commander.
BattleSlots is used for determining the size of the deployslot array, which represents the number of units the commander can take into battle based on certain other members in Commander.

UnitSlots defaults to 48 and BattleSlots ranges from 6-12. The deployslots array was running over its allotted memory. Since linux spaces memory allocation differently, it wasn't causing a problem, but because windows crams data together it was running over.
Silly me.

[Athar]

Ok, I've recently started learning C++ based on the "Teach yourself C++ in 21 Days" book, which btw I highly recommend for anyone starting C++ from scratch.

And I highly discourage getting that book. It teaches very poor style and the author clearly is not an experienced C++ programmer.
Some of this bad style already carried over to your own code:

For example:
char name[21];

Using a char array instead of a string is a really bad idea. Not only is string manipulation very complicated and awkward when you use char arrays, but here you also unnecessarily limit the max length of name to 20 characters, which requires additional checking that the name does not exceed that length, otherwise you can get a buffer overflow. The correct way is to use std::string, which has none of these problems.
The second issue is

Unit ** deployslot;
Unit ** inventoryslot;

This has the problem that you need to do manual memory management and that you can't easily resize the array as needed.
A vector<Unit*> would be the better solution. It is an implementation of an dynamic array that handles memory management and resizing for you. It even makes debugging easier. With the global macro _GLIBCXX_DEBUG defined, the program would have immediately alerted you that you tried to access an invalid index in deployslot and so you would've found the problem in just a minute.

You could read Thinking in C++, which is freely available on the web and is better than Liberty's book.

[jovispater]

Actually, the things I had trouble with I didn't learn in this book. I actually haven't finished it. It does recommend strings instead of char arrays once I got to that chapter, as well as teach about vectors, but I wrote this program before getting to those chapters, and instead used quickie Google searches to fill in the gaps in my knowledg to get what I wanted done. The reason I recommend the book is because I bought several C++ "beginner level" books last year but couldn't make heads or tails of them as a true novice who didn't even know what a compiler was. This book jumpstarted me in the right direction and gave me enough of a foundation that I can now go back and figure out what the other books were trying to convey to me.

In any case, I will finish it soon and will be looking to expand my knowledge through more advanced books, and will definitely look into any recommendations. I've just begun dabbling with Qt4 and would like to eventually pursue a higher education in the field of programming when I get out of active duty in the near future. In the mean time, I'm also looking for recommendations on perhaps some kind of C++ workbook for practice. For me, repitition is the mother of learning.