Program crashes at very end

Hello all ... could you please look over this code, critique it and let me know why it crashes at the end?

TIA!

main.cpp

Code:

---

#include <iostream>
#include <string>
#include "World.h"
#include "Player.h"

using namespace std;

int main(){

  World world;

  cout << "World Name = " << world.getName() << endl;

  char* pName;

  cout << "What is your name?" << endl;

  cin >> pName;

  Player player;

  player.setName(pName);

  cout << player.getName() << endl;

  return 0;
}

---

player.h

Code:

---

#ifndef PLAYER_H
#define PLAYER_H
  class Player {
    public:

      char* getName();
      void setName(char* _pstrPlayerName);

    protected:

      char* _pstrPlayerName;

  };
#endif

---

player.cpp

Code:

---

#include <string>
#include "Player.h"

void Player::setName(char* playerName){

  _pstrPlayerName = playerName;

}

char* Player::getName(){

  return _pstrPlayerName;

}

---

world.h

Code:

---

#ifndef WORLD_H
#define WORLD_H

class World {

  public:

    World();
    char* getName();

  protected:

    char* _pstrWorldName;

};

#endif

---

world.cpp

Code:

---

#include "World.h"

World::World(){

  _pstrWorldName = "Something";

}

char* World::getName(){

  return _pstrWorldName;

}

---

Output:

Code:

---

World Name = Something
What is your name?
Stuff

Process returned -1073741819 (0xC0000005)  execution time : 14.163 s
Press any key to continue.

---

If I comment out the two lines referring to World, it runs with no errors.

A pointer is really just a glorified integer. It holds the address of some piece of data. Before you set it to some value, you need to consider the lifetime and ownership of the object it is pointing to, and above all it must be pointing somewhere valid at all times (in this context NULL is valid).

>> char* pName;

This variable points to nowhere in particular. Assign it to some chunk of memory.

>> _pstrPlayerName = playerName;

What is the lifetime of the object that was passed to the function, and who owns it? Make a copy.

I created a NULL pointer out of pName and changed the assignment line to

Code:

---

void Player::setName(char* playerName){

  strcpy(_pstrPlayerName, playerName);

}

---

and it still crashes. :\

>> I created a NULL pointer out of pName and changes the assignment line to

Ok, so now the variable hold the address starting at the 0th offset in ram. You send the variable to strcpy and it attempts to write some data there. The problem is, your program doesn't have any business with that address in memory! You need to allocate some yourself, either on the stack or on the heap.

Ok, I see what you're saying.

Is there a way to dynamically expand the memory allocated to a char pointer to prevent a buffer over flow on:

Code:

---

char* pName = new char[10];

---

You can dynamically expand a block of data by allocating a bigger block and copying (then delete the old one). std::string does this for you.

This is roughly the minimum "standard" implementation you might have. Notice that it is *extremely* important that you define a copy constructor and operator= if you have a class containing pointers (which is one reason why they should be avoided at all costs!):

Code:

---

  class Player {
    public:
      Player();
      Player(const Player& rhs);
      Player& operator =(const Player& rhs);
      char* getName() const;
      void setName(char* _pstrPlayerName);
      virtual ~Player();
    protected:
      void cleanup();
      char* _pstrPlayerName;

  };


Player::Player() : _pstrPlayerName(NULL){

  setName("");

} 
 
Player::Player(const Player& rhs) : _pstrPlayerName(NULL){

  *this = rhs;

} 
 
Player& Player::operator =(const Player& rhs){

  setName(rhs.getName());
  return *this;

} 
 
void Player::setName(char* playerName){

  cleanup();
  _pstrPlayerName = new char[strlen(playerName) + 1];
  strcpy(_pstrPlayerName, playerName);

}

char* Player::getName() const {

  return _pstrPlayerName;

}

void Player::cleanup(){

  delete [] _pstrPlayerName;
  _pstrPlayerName = NULL;

}

Player::~Player(){

  cleanup();

}

---

Of course this would be much more simple using an std::string. Notice how the minimum standard for non-pointer containing classes is much less stringent:

Code:

---

  class Player {
    public:
      string getName() const;
      void setName(const string& _pstrPlayerName);
    protected:
      string _pstrPlayerName;

  };

void Player::setName(const string& playerName){

  _pstrPlayerName = playerName;

}

string Player::getName() const {

  return _pstrPlayerName;

}

---

Big difference, no?

Quote:

---

Originally Posted by Sebastiani

Big difference, no?

---

Just a little bit.

Ok, I got it working with no errors with:

Code:

---

#ifndef PLAYER_H
#define PLAYER_H
  class Player {
    public:

      const std::string getName();
      void setName(const std::string& _strPlayerName);

    protected:

      std::string _strPlayerName;

  };
#endif

---

Code:

---

#include <string>
#include "Player.h"

void Player::setName(const std::string& playerName){

  _strPlayerName = playerName;

}

const std::string Player::getName(){

  return _strPlayerName;

}

---

Code:

---

#include <iostream>
#include <string>
#include "World.h"
#include "Player.h"

using namespace std;

int main(){

  World world;

  cout << "World Name = " << world.getName() << endl;

  string pName = "";

  cout << "What is your name?" << endl;

  cin >> pName;

  Player player;

  player.setName(pName);

  cout << player.getName() << endl;

  return 0;
}

---

I had to put std:: in front of each string. Thanks for your help!

Just for future reference...

Code:

---


  class Player {
  typedef std:string string;
  public:

      const string getName();
      void setName(const string& _strPlayerName);

    protected:

      string _strPlayerName;

  };

---

... also works without changing your code too terribly.

I think you meant typedef std::string string ?

Quote:

---

.. also works without changing your code too terribly.

---

No, that gives you a compile error. I think you meant:

Code:

---

typedef std::string string

---

That is a bad idea though because if you ever add something like

Code:

---

using namespace std;

---

you will now get another compile error.

The best way to avoid explicitly adding std:: before each string declaration would be to do:

Code:

---

using std::string;
class Player {
  public:
      const string getName();
      void setName(const string& _strPlayerName);
    protected:
      string _strPlayerName;
  };

---

Code:

---

using std::string;
class Player {
  public:
      const string getName();
      void setName(const string& _strPlayerName);
    protected:
      string _strPlayerName;
  };

---

Is ok on a small project. If you are building something larger I would urge you to utilize namespaces.

Code:

---

namespace UI
{
  using std::string;
  class Player;
}

---

Which prevents ambiguity across your entire program.

And if you want to get really fancy about it you could employ something like this:

Code:

---

struct default_player_traits
{
        typedef std::string
                string_type;
        // ...etc...
};

template < typename Traits = default_player_traits >
class BasicPlayer
{
        public:
       
        typedef typename Traits::string_type
                string;
        // ...etc...
};

typedef BasicPlayer< >
        Player;

---

Traits too? You really do love template solutions, don't you? ;)

>> Traits too? You really do love template solutions, don't you?

As a matter of fact, I do. :)

The cool thing about traits is that you can write an entire library code and include not a single header file! As an example, I wrote a large integer class that provided full support for I/O and strings, and even allowed assignment/construction from arbitrary types (such as string) - all without any header files whatsoever. So yes, it's extremely flexible, and well worth the effort in many cases.