Bloqueo al eliminar a través del destructor

8

En el siguiente programa, tengo la intención de copiar char* linecontenidos de un objeto a otro strcpy. Sin embargo, cuando finaliza el programa, el destructor de obj2trabajos funciona bien pero el dtor de los objbloqueos. gdb muestra diferentes direcciones de lineambos objetos.

class MyClass {
        public:
                char *line;
                MyClass() {
                        line = 0;
                }
                MyClass(const char *s) {
                        line = new char[strlen(s)+1];
                        strcpy(line, s);
                }
                ~MyClass() {
                        delete[] line;
                        line = 0;
                }
                MyClass &operator=(const MyClass &other) {
                        delete[] line;
                        line = new char[other.len()+1];
                        strcpy(line, other.line);
                        return *this;
                }
                int len(void) const {return strlen(line);}
};

int main() {
        MyClass obj("obj");
        MyClass obj2 = obj;
anurag86
fuente
Aunque use cadenas terminadas en nulo estilo C, todavía está programando en C ++.
Algún programador amigo
55
También necesitas un constructor de copias. Regla de tres
ChrisMM
Esto es así porque me pidieron que simulan cadena de copia en C ++ a través strcpy
anurag86
2
Solo como un aparte, una vez que agregue un constructor de copia: MyClass obj1; MyClass obj2 = obj1;seguirá segfault porque llamará a strlen(obj1.line)cuál es strlen(NULL). Como lo haría MyClass obj1; obj1.len();.
Bill Lynch el
2
También comportamiento indefinido: MyClass obj1; obj1.len(); es un comportamiento indefinido invocar strlenun puntero nulo.
PaulMcKenzie

Respuestas:

13

Con

MyClass obj2 = obj;

no tienes asignación, tienes copia de construcción . Y no sigue las reglas de tres, cinco o cero ya que no tiene un constructor de copia, por lo que el generado por defecto simplemente copiará el puntero.

Eso significa que después de esto tiene dos objetos cuyo linepuntero apunta a la misma memoria. Eso conducirá a un comportamiento indefinido una vez que uno de los objetos se destruya, ya que deja al otro con un puntero no válido.

La solución ingenua es agregar un constructor de copia que haga una copia profunda de la cadena en sí, de manera similar a lo que está haciendo su operador de asignación.

Una mejor solución sería utilizar std::stringen su lugar sus cadenas y seguir la regla de cero.

Algún tipo programador
fuente
4

Necesita crear un constructor de copia. Esto tiene que hacer la regla de 3/5 . Está creando obj2, lo que significa que se invoca un constructor de copia, no el operador de asignación de copia.

Debido a que no tiene un constructor de copia, se realiza una copia "superficial". Esto significa que linese copia por valor. Ya que es un puntero, tanto objy obj2están apuntando a la misma memoria. Se llama al primer destructor y borra esa memoria bien. Se llama al segundo constructor y se produce una doble eliminación, lo que causa su falla de segmentación.

class MyClass {
public:
  char *line = nullptr;
  std::size_t size_ = 0;  // Need to know the size at all times, can't 
                          // rely on null character existing
  const std::size_t MAX_SIZE = 256;  // Arbitrarily chosen value
  MyClass() { }
  MyClass(const char *s) : size_(strlen(s)) {
    if (size_ > MAX_SIZE) size_ = MAX_SIZE;
    line = new char[size_];
    strncpy(line, s, size_ - 1);  // 'n' versions are better
    line[size_ - 1] = '\0';
  }
  MyClass(const MyClass& other) : size_(other.size_) {  // Copy constructor
    line = new char[size_ + 1];
    strncpy(line, other.line, size_);
    line[size_] = '\0';
  }
  ~MyClass() {
    delete[] line;
    line = nullptr;
  }
  MyClass& operator=(const MyClass &other) {
    if (line == other.line) return *this;  // Self-assignment guard
    size_ = other.size_;
    delete[] line;
    line = new char[other.size_ + 1];
    strncpy(line, other.line, size_);
    line[size_] = '\0';
    return *this;
  }
  int len(void) const { return size_; }
};

Cuando se trata de C-Strings, absolutamente no puede perder el carácter nulo. El problema es que es extremadamente fácil perder. También le faltaba una protección de autoasignación en su operador de asignación de copias. Eso podría haberte llevado a bombardear accidentalmente un objeto. Agregué un size_miembro y lo usé en strncpy()lugar de strcpy()porque poder especificar un número máximo de caracteres es increíblemente importante en el caso de perder un carácter nulo. No evitará el daño, pero lo mitigará.

Hay otras cosas que me gustaron usando la Inicialización de miembro predeterminada (a partir de C ++ 11) y el uso de una lista de inicialización de miembro constructor . Mucho de esto se vuelve innecesario si puede usarlo std::string. C ++ puede ser "C con clases", pero vale la pena tomarse el tiempo para explorar realmente lo que el lenguaje tiene para ofrecer.

Algo que un constructor y destructor de copias de trabajo nos permite hacer es simplificar nuestro operador de asignación de copias utilizando el "modismo de copia e intercambio".

#include <utility>

MyClass& operator=(MyClass tmp) { // Copy by value now
  std::swap(*this, tmp);
  return *this;
}

Enlace a la explicación .

sweenish
fuente
2
Una mejor solución sería una implementación que usa el idioma de copiar y cambiar.
The Philomath
1
Aprendí algo nuevo, y me gusta. Agregaré un poco más.
Sweenish
Gracias por copiar usando el ejemplo de intercambio
anurag86