Falha ao excluir através do destruidor

8

No programa a seguir, pretendo copiar o char* lineconteúdo de um objeto para outro strcpy. No entanto, quando o programa termina, o destruidor de obj2obras funciona bem, mas o dtor de objfalhas. O gdb mostra endereços diferentes linepara os dois 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
fonte
Mesmo que você use seqüências terminadas em nulo no estilo C, ainda está programando em C ++.
Algum programador
5
Você também precisa de um construtor de cópias. Regra de três
ChrisMM 25/11/19
Isso é porque me pediram para cordas cópia simular em c ++ através strcpy
anurag86
2
Apenas como um aparte, uma vez que você adiciona um construtor de cópia: MyClass obj1; MyClass obj2 = obj1;ainda irá falhar porque você chamará strlen(obj1.line)qual é strlen(NULL). Como seria MyClass obj1; obj1.len();.
Bill Lynch
2
Comportamento também indefinido: MyClass obj1; obj1.len(); é um comportamento indefinido para chamar strlenum ponteiro nulo.
PaulMcKenzie

Respostas:

13

Com

MyClass obj2 = obj;

você não tem atribuição, você tem cópia-construção . E você não segue as regras de três, cinco ou zero, pois não possui um construtor de cópias; portanto, o gerador gerado por padrão apenas copia o ponteiro.

Isso significa que, depois disso, você terá dois objetos cujo lineponteiro apontará exatamente para a mesma memória. Isso levará a um comportamento indefinido quando um dos objetos for destruído, deixando o outro com um ponteiro inválido.

A solução ingênua é adicionar um construtor de cópia que faça uma cópia profunda da própria string, da mesma forma que o operador de atribuição está fazendo.

Uma solução melhor seria usar std::stringsuas strings e seguir a regra do zero.

Algum cara programador
fonte
4

Você precisa criar um construtor de cópias. Isso tem que fazer a regra de 3/5 . Você está criando obj2, o que significa que um construtor de cópias é chamado, não o operador de atribuição de cópias.

Como você não possui um construtor de cópias, é feita uma cópia "superficial". Isso significa que lineé copiado pelo valor. Como é um ponteiro, ambos obje obj2estão apontando para a mesma memória. O primeiro destruidor é chamado e apaga bem essa memória. O segundo construtor é chamado e ocorre uma exclusão dupla, causando sua falha de segmentação.

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_; }
};

Ao lidar com C-Strings, você absolutamente não pode perder o caractere nulo. A questão é que é extremamente fácil perder. Você também não tinha um protetor de atribuição automática no operador de atribuição de cópias. Isso poderia ter levado você a acidentalmente destruir um objeto. Eu adicionei um size_membro e usei, em strncpy()vez de, strcpy()porque poder especificar um número máximo de caracteres é incrivelmente importante no caso de perder um caractere nulo. Não evitará danos, mas os mitigará.

Há outras coisas que eu gostei de usar a Inicialização Padrão de Membros (a partir do C ++ 11) e a lista de inicialização de membros construtores . Muito disso se torna desnecessário se você puder usar std::string. C ++ pode ser "C com classes", mas vale a pena dedicar um tempo para realmente explorar o que a linguagem tem a oferecer.

Algo que um construtor e destruidor de cópias de trabalho nos permite fazer é simplificar nosso operador de atribuição de cópias usando o "idioma de copiar e trocar".

#include <utility>

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

Link para explicação .

sweenish
fonte
2
Uma solução melhor seria uma implementação usando o idioma de copiar e trocar.
The Philomath
1
Aprendi uma coisa nova, e eu gosto. Vou adicionar um pouco mais.
sweenish 25/11/19
Obrigado por cópia usando exemplo de swap
anurag86