Jump to content
  • Advertisement
Sign in to follow this  
blueshogun96

std::string causing crashes!

This topic is 2155 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

In the past, I've gotten flak for using "unsafe" C-strings.  In C++, std::string is giving me a headache.  Randomly, when using a C++ string, it will cause my game to crash.  Maybe it's something simple, but honestly, I've never had this problem before.  It only happens when I try to set one string variable equal to another, like so:

void game_app_t::add_new_hint( std::string str )
{
    struct hint_t* h = (struct hint_t*) malloc( sizeof( struct hint_t ) );
    
    /* Add a new hint to the screen */
    h->pos = m_user.pos;
    h->str = str;
    h->timer = 60;
    h->alpha = 255.0f;
    h->pos.v[0] += 32.0f;
    
    list_add_beginning( &m_hints, h );
}

/* Example usage */
add_new_hint( "Avoid this" );

I've had this problem with random crashes before, and I ended up saying "screw it, I'll use C-strings", and that was the end of the crashes.  Now it's happening again, and I'd rather find a solution as opposed to another work around fix.  Any ideas?  Thanks.

 

Shogun.

 

EDIT: Not sure if it matters, but I'm using XCode and this game is being dev'ed on Mac OSX Lion.

Edited by blueshogun96

Share this post


Link to post
Share on other sites
Advertisement

In addition to what Brother Bob said, never use malloc in C++. Not only does new ensure that constructors are called, it's also more type safe and the C++ way of allocating memory.

Share this post


Link to post
Share on other sites

Try to avoid both `malloc' and `new'. If you are adding something to a container, use a standard container (of objects, not pointers) and don't even mention explicit memory allocation in your code.

Here's what a more idiomatic C++ version of your code could look like:

struct Hint {
  Vector2D pos;
  std::string str;
  int timer;
  float alpha;

  Hint(Vector2D const &pos, std::string const &str, int timer, float alpha)
    : pos(pos),
      str(str),
      timer(timer),
      alpha(alpha) {
  }
};

class GameApp {
  std::vector<Hint> hints;
  User user;

public:
  void add_new_hint(std::string const &str) {
    hints.push_back(Hint(user.pos + Vector2D(32.0f, 0.0f), str, 60, 255.0f));
  }
  
  // ...
};

If this is running on a C++11 compiler I would use an emplace_back in this case because it will avoid the copy construction in push_back, but only in the case where you construct the object on the push_back call.

Share this post


Link to post
Share on other sites

If this is running on a C++11 compiler I would use an emplace_back in this case because it will avoid the copy construction in push_back, but only in the case where you construct the object on the push_back call.

 

Hmmm... I tried that on g++ 4.7.1 and it doesn't seem to avoid the copy. Perhaps I am not doing it right? I compiled it with

g++ kk.cpp -o kk -O3 -Wall -Wextra -std=c++11
#include <iostream>
#include <vector>

struct Vector2D {
  float x, y;
 
  Vector2D(float x, float y) : x(x), y(y) {
  }
};

Vector2D operator+(Vector2D const &v1, Vector2D const &v2) {
  return Vector2D(v1.x + v2.x, v1.y + v2.y);
}

struct Hint {
  Vector2D pos;
  std::string str;
  int timer;
  float alpha;

  Hint(Vector2D const &pos, std::string const &str, int timer, float alpha)
    : pos(pos),
      str(str),
      timer(timer),
      alpha(alpha) {
  }
 
  Hint(Hint const &other)
    : pos(other.pos),
      str(other.str),
      timer(other.timer),
      alpha(other.alpha) {
    std::cerr << "A hint is being copied!\n";
  }
};

struct User {
  Vector2D pos;
 
  User(Vector2D const &pos) : pos(pos) {
  }
};

class GameApp {
  std::vector<Hint> hints;
  User user;

public:
  void add_new_hint(std::string const &str) {
    hints.emplace_back(Hint(user.pos + Vector2D(32.0f, 0.0f), str, 60, 255.0f));
  }
 
  GameApp(User const &user) : user(user) {
  }
};

int main() {
  GameApp game_app(User(Vector2D(1.0f, 2.0f)));
  game_app.add_new_hint("A hint!");
}

EDIT: Oh, I got it. If there is a move constructor, it does use it. I added this one and it works fine:

  Hint(Hint &&other)
    : pos(std::move(other.pos)),
      str(std::move(other.str)),
      timer(std::move(other.timer)),
      alpha(std::move(other.alpha)) {
    std::cerr << "A hint is being moved!\n";
  }

FURTHER EDIT: This also works:

  Hint(Hint &&) = default;
Edited by Álvaro

Share this post


Link to post
Share on other sites

 


If this is running on a C++11 compiler I would use an emplace_back in this case because it will avoid the copy construction in push_back, but only in the case where you construct the object on the push_back call.

 

Hmmm... I tried that on g++ 4.7.1 and it doesn't seem to avoid the copy. Perhaps I am not doing it right? I compiled it with

emplace_back forwards the parameters to a constructor, so if you construct a Hint object and pass it to emplace_back, it would then have to pass that object to the copy constructor. This one does not invoke the copy constructor in VS2012.

hints.emplace_back(user.pos + Vector2D(32.0f, 0.0f), str, 60, 255.0f);

Share this post


Link to post
Share on other sites

 

 


If this is running on a C++11 compiler I would use an emplace_back in this case because it will avoid the copy construction in push_back, but only in the case where you construct the object on the push_back call.

 

Hmmm... I tried that on g++ 4.7.1 and it doesn't seem to avoid the copy. Perhaps I am not doing it right? I compiled it with

emplace_back forwards the parameters to a constructor, so if you construct a Hint object and pass it to emplace_back, it would then have to pass that object to the copy constructor. This one does not invoke the copy constructor in VS2012.

hints.emplace_back(user.pos + Vector2D(32.0f, 0.0f), str, 60, 255.0f);

 

I see. That works on my compiler as well.

 

By the way, push_back won't make a copy either if you provide a move constructor.

  Hint(Hint &&) = default;

Share this post


Link to post
Share on other sites

Okay, now I get it.  It's amazing what I was never taught in my C++ courses! ^o^

 

My linked list library was originally written in pure C, and I was too lazy to change malloc/free to new/delete.  I usually prefer to avoid using std C++ libraries in my code, but I made an exception this time, and it turned out to be a major mistake portability wise (I'll never forget the nightmare I had dealing with std C++ and Blackberry OS).  Oh well.

 

Thanks.

 

Shogun.

Edited by blueshogun96

Share this post


Link to post
Share on other sites
By the way, push_back won't make a copy either if you provide a move constructor.
  Hint(Hint &&) = default;

That is still more wasteful than using emplace-back, since first a Hint-instance is created, and then the move constructor is called for the Hint-instance in the vector. emplace_back only calls the constructor once for the vector instance. Specially here, since all members are POD except for the std::string, having a move-constructor won't help much, move constructor normally only helps when custom memory allocations are made in the class. Not that the difference is a big deal here anyway, but just wanted to add this.

Edited by Juliean

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!