Ошибка сегмента после того, как элемент помещен в контейнер STL - PullRequest
Новая
       20

Ошибка сегмента после того, как элемент помещен в контейнер STL

4 голосов
/ 24 марта 2009 
typedef struct temp  
{  
        int a,b;  
        char  *c;  
        temp(){ c = (char*)malloc(10);};  
        ~temp(){free(c);};  
}temp;  

int main()  
{  
   temp a;  
   list<temp>   l1;  
   l1.push_back(a);  
   l1.clear();  
   return 0;  

}

дает ошибку сегментации.

Ответы [ 10 ]

27 голосов
/ 24 марта 2009

У вас нет конструктора копирования.

Когда вы нажимаете 'a' в списке, он копируется. Поскольку у вас нет конструктора копирования (чтобы выделить память для c и скопировать из старого c в новый c), c - это тот же указатель в a и копия a в списке.

Деструктор для обоих вызывается, первый будет успешным, второй потерпит неудачу, поскольку память, на которую указывает c, уже освобождена.

Вам нужен конструктор копирования.

Чтобы увидеть, что происходит, вставьте несколько контуров в конструкторы и деструкторы и пошагово пройдитесь по коду.

5 голосов
/ 24 марта 2009

Вам нужен конструктор глубокой копии, чтобы избежать двойного освобождения (). У вас есть переменная temp class ( a ), затем вы добавляете ее в список. Переменная копируется. Затем вы очищаете список, элемент внутри уничтожается и вызывается free (). Затем переменная a уничтожается и снова вызывается free () для того же адреса, что приводит к ошибке сегментации.

Вам нужен конструктор копирования для глубокого копирования временных переменных класса, которые будут вызывать malloc () другой буфер и копировать данные.

3 голосов
/ 24 марта 2009

Помимо приведенных исправлений, вы должны избегать использования malloc / free в C ++. В вашем конкретном случае я бы пошел с вектором: 

#include <vector>

 typedef struct temp  
{  
        int a,b;  
        std::vector<char> c;  
        temp(){ c.reserve(10);};  
}temp;  

int main()  
{  
   temp a;  
   list<temp>   l1;  
   l1.push_back(a);  
   l1.clear();  
   return 0;  

}
3 голосов
/ 24 марта 2009

В тот момент, когда вы звоните l1.push_back(a), создается вторая копия «а». В результате теперь есть два класса, которые считают, что они владеют памятью от исходного вызова malloc, и когда второй удален, он попытается освободить память, удаленную первым.

Решение состоит в том, чтобы добавить конструктор копирования, который учитывает тот факт, что экземпляр класса на самом деле не владеет данными. Как правило, вы делаете это, имея некоторую форму подсчета ссылок.

1 голос
/ 24 марта 2009

В дополнение к конструктору копирования в этом случае целесообразно также указать оператор =. 

struct temp {   // typedef is implicit in C++
  int a,b;
  char * c;

  // Constructor
  temp() { c = malloc(10); }
  // Destructor
  ~temp() { free(c); }
  // Copy constructor
  temp(const temp & x) { c = malloc(10); setTo(x); }
  // Operator =
  temp & operator = (const temp & x) { setTo(x); return *this; }

  // Initialize THIS to X
  void setTo(const temp & x) { a = x.a; b = x.b; memcpy(c,x.c,10); }
};
1 голос
/ 24 марта 2009

Если вы не хотите добавлять конструктор копирования, вы можете рассмотреть список указателей на значения вместо списка значений. 

list<temp*>   l1;
l1.push_back( new temp() );

Но тогда вам нужно вручную удалить каждый объект в списке, чтобы предотвратить утечку памяти.

Также члены a, b в вашей структуре не инициализируются. Будьте осторожны.

0 голосов
/ 24 марта 2009

Как иронично, что у этого был тег "STL", но отсутствие STL - вот что вызывает проблему.

0 голосов
/ 24 марта 2009

Другая проблема с этим кодом - дополнительные точки с запятой в 

temp(){ c = (char*)malloc(10);};
~temp(){free(c);};

лучше их убрать: 

temp(){ c = (char*)malloc(10);}
~temp(){free(c);}
0 голосов
/ 24 марта 2009

Вам необходимо определить конструктор копирования для temp. Прямо сейчас, то, что происходит, когда вы нажимаете a в список, создается копия a. Копия (назовите ее a2) инициализируется, сказав a2.c = a.c. Это означает, что и a, и a2 указывают на один и тот же блок памяти. Когда вызываются их деструкторы, этот блок памяти освобождается дважды, что приводит к ошибке сегментации.

Учитывая то, что вы опубликовали, конструктор копирования, вероятно, должен выглядеть примерно так: 

temp::temp (temp const &rhs)
{
    this->a = rhs.a;
    this->b = rhs.b;
    this->c = (char *) malloc (10);
    memcpy (this->c, rhs.c, 10);
}

Это предполагает, что то, на что указывает c, всегда имеет длину 10 символов ...

0 голосов
/ 24 марта 2009

Вам нужен конструктор копирования и оператор присваивания - вещи, хранящиеся в коллекции STL, хранятся в виде копий и могут быть переназначены при изменении размера вектора.

Трудно понять из вашего кода, какой именно должна быть семантика конструктора копирования, но как минимум достаточно выделить некоторое количество памяти, чтобы деструктор (если не что-то еще) мог что-то освободить. Оператор присваивания одинаково трудно определить без более подробной информации о вашем классе.

...