Memory leak caused by function in C++

Question

there is a function which creates new memory and is used in constructor. However, I do not know how to access it in destructor to delete it.

In constructor is used (same file) player.cpp

#include <string.h>

#include "Player.h"

// TODO: Fix the bugs in this file

Player::Player(const char* name) : name_(0)
{
  copyString(&name_, name);
}

Player::Player(const Player& copy) : name_(0)
{
  name_ = copy.name_;
}

Player::~Player()
{ 
  delete [] name_; // Not sure if it works. errors promts- double free
}

Player& Player::operator=(const Player& copy)
{
  return *this;
}

void Player::copyString(char** dest, const char* source)
{
  unsigned int str_len = strlen(source);
  char* str = new char[str_len+1]; //This line
  strncpy(str, source, str_len);
  str[str_len] = '\0';

  *dest = str;
}

std::ostream& operator<<(std::ostream& out, const Player& player)
{
  out << player.name_ << std::endl;
  return out;
}

All i am allowed to change are .cpp files. I have added delete line i destructor, but error shows up.

`delete [] name_;`? You'd have to post more of your code, including your class definition for a more informed answer. — Retired Ninja, Oct 24 '17 at 18:41
If `name_` points to that memory, can't you `delete[] name_;`? Consider using `std::string` in actual projects, and smart pointers instead of raw owning pointers. — François Andrieux, Oct 24 '17 at 18:41
Looks like you have (or need) a custom destructor. Some heads up reading to try and stave off what's typically the next couple bugs: [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — user4581301, Oct 24 '17 at 18:54
@RetiredNinja i have done as you suggested, but error shows up. 'double free' — Ryad Kovach, Oct 24 '17 at 18:56
Your assignment operator and copy constructor are incorrect and are only copying the pointer. If you want to do it correctly you will need to make a copy of the data. https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three https://stackoverflow.com/questions/14063791/double-free-or-corruption-but-why — Retired Ninja, Oct 24 '17 at 18:59
Right on! Rule of Three was accounted for! Bit of a bug in the implementation, but easily solved with `copyString`. — user4581301, Oct 24 '17 at 19:04
You can remove these memory allocation issues by using `std::string`. — Thomas Matthews, Oct 24 '17 at 19:32

mksteve · Answer 1 · 2017-10-24T19:20:10.353

The copy constructor needs to do a deep copy, otherwise there are 2 things pointing at the same place...

Player::Player(const Player& copy) : name_(0)
{
  copyString(&name_, copy.name_);
}

The operator= does not work - just ignores the old value, so should also do a copy....

Player& Player::operator=(const Player& copy)
{
  if( this != & copy ){
      delete [] name_;
      copyString(&name_, copy.name_);
  }
  return *this;
}

You already have a delete [] in the destructor.

The rule of 3 (or 0) is a rule about how to manage resources. If you implement a destructor, a copy constructor, or an operator=, then you probably need to implement all three.

If you don't then, you will be OK with none of them (the rule of 0). The thinking is that if a resource needs attention at copy, construct or destruct, then make it into an individual class which is well considered for all three behaviors.

If you create basic classes (like this "string" class - Player), then things containing Player can live with no knowledge of the semantics of the copy, and safe in the knowledge that resources are managed for them.

Consider mentioning http://en.cppreference.com/w/cpp/language/rule_of_three — François Andrieux, Oct 24 '17 at 19:15

Memory leak caused by function in C++

1 Answers1