0

I have a msg class which holds a char* and its size basically. And sometimes I get a SIGABRT when deleting my char* and I can't figure out why! 

class MSG
{
private:
  char* data;
public:
  std::size_t size;
  std::string target_id;

MSG( const MSG& msg ) : MSG( msg.getData(), msg.size, msg.target_id )
{ };

MSG( char* sourceData, std::size_t sourceSize, const std::string& id )
{
  data = new char[sourceSize];
  std::copy_n( sourceData, sourceSize, data );
  size = sourceSize;
  target_id = id;
};

MSG( ) : MSG( ( char* ) nullptr, 0, UNDEFINED )
{ };

~MSG( )
{
     delete[] data;
     data = 0;
};

MSG& operator=(MSG& that)
{
  swap(*this, that);
  return *this;
}

MSG& operator=(const MSG& other)
{
  if (this != &other) // (1)
  {
     // get the new data ready before we replace the old
     std::size_t newSize = other.size;
     char* newArray = newSize ? new char[newSize]() : nullptr; // (3)
     std::copy(other.getData(), other.getData() + newSize, newArray); // (3)

     // replace the old data (all are non-throwing)
     delete [] data;
     size = newSize;
     data = newArray;
     target_id = other.target_id;
  }

  return *this;
}
};

When debugging I get this output which leads to the error:

data = {char} 0 '\000'
size = 2

I really don't know what's wrong here...

Update: In terminal I get more info: double free or corruption

2nd Update: Added copy constructor

3rd update: Added assignment operators

madmax
  • 1,803
  • 3
  • 25
  • 50
  • Classic error, failing to follow the rule of three. Basically when you copy your MSG object, two objects end up with the same `data` pointer and both try to delete it resulting in double deletion error. – john Sep 11 '18 at 11:16
  • 1
    Looks like `data` is null when you try to delete it. If you default construct MSG, there is nothing allocated, and nothing to free. – Jens Luedicke Sep 11 '18 at 11:17
  • 1
    @JensLuedicke deleting null is not an error. It's explicitly allowed and does nothing. – john Sep 11 '18 at 11:18
  • @john added the copy constructor. And as I make a copy, I don't have two objects with same data pointer or? – madmax Sep 11 '18 at 11:36
  • 1
    @madmax Well you need an assignment operator as well. If you are still getting the problem after adding both of those, then you should add the new code to the question. – john Sep 11 '18 at 11:42
  • 1
    @madmax Just in case it's not clear, the point is that you write your copy constructor and assignment operator to ensure that the copied MSG object does not end up with the same `data` pointer as the original. Or you take the easy route and use a `std::string` instead of a `char*`, in which case you don't need to define a copy constructor and assignment operator. – john Sep 11 '18 at 11:44
  • Yes, thank you, I forgot the assignment operator!! Added it to the question. But how would a const assignment operator look like when I want to use swap? My current one is a bit ugly... – madmax Sep 11 '18 at 12:28

0 Answers0