Segfault Copy Constructor

Question

Мой код выглядит следующим образом:

void Scene::copy(Scene const & source)    
{
maxnum=source.maxnum;
imagelist = new Image*[maxnum];

for(int i=0; i<maxnum; i++)
{
   if(source.imagelist[i] != NULL)
   {
    imagelist[i] = new Image;
    imagelist[i]->xcoord = source.imagelist[i]->xcoord;
    imagelist[i]->ycoord = source.imagelist[i]->ycoord;
    (*imagelist[i])=(*source.imagelist[i]);
   }

   else
   {
   imagelist[i] = NULL;
   }
}
}

Небольшая предыстория: класс Scene имеет закрытое int, называемое maxnum, и динамически размещаемые указатели Array of Image при создании. Эти указатели указывают на изображения. Конструктор копирования пытается сделать глубокую копию всех изображений в массиве. Каким-то образом я получаю Segfault, но не понимаю, как бы я мог получить доступ к массиву за пределами.

Кто-нибудь видит что-то не так?

Я новичок в C ++, так что, возможно, это что-то очевидное.

Спасибо

Answer 1

Если исходное изображение имеет значение maxnum, превышающее фактическое количество элементов в его списке изображений, то цикл будет выполняться после конца массива source.imagelist. Возможно, maxnum инициализируется значением 1, в то время как массив начинается пустым (или maxnum может вообще не инициализироваться), или, если у вас есть функция Scene :: remove_image (), он мог удалить запись списка изображений без уменьшения MAXNUM. Я бы предложил использовать std :: vector вместо необработанного массива. Вектор будет отслеживать свой собственный размер, поэтому ваш цикл for будет:

for(int i=0; i<source.imagelist.size(); i++)

и он получит доступ только к тому количеству элементов, которое содержится в исходном векторе. Другое возможное объяснение сбоя заключается в том, что один из ваших указателей в source.imagelist принадлежит удаленному изображению, но указатель никогда не был установлен в NULL и теперь является висящим указателем.

delete source.imagelist[4];
...
...  //  If source.imagelist[4] wasn't set to NULL or removed from the array,
...  //  then we'll have trouble later.
...
for(int i=0; i<maxnum; i++)
{
    if (source.imagelist[i] != NULL)  //  This evaluates to true even when i == 4
    {
        //  When i == 4, we're reading the xcoord member from an Image
        //  object that no longer exists.
        imagelist[i]->xcoord = source.imagelist[i]->xcoord;

Эта последняя строка будет обращаться к памяти, чего не должно быть. Возможно, объект все еще существует в памяти, потому что он еще не был перезаписан, или, возможно, он был перезаписан, и вы получите недопустимое значение xcoord. Если вам повезет, то ваша программа просто рухнет. Если вы имеете дело непосредственно с new и delete, убедитесь, что вы установили указатель на NULL после его удаления, чтобы у вас не было висящего указателя. Это не предотвращает эту проблему, если вы храните копию указателя где-то, однако, в этом случае вторая копия не будет установлена ​​в NULL, когда вы удаляете-и-NULL первую копию. Если вы позже попытаетесь получить доступ ко второй копии указателя, у вас не будет возможности узнать, что он больше не указывает на действительный объект.

Гораздо безопаснее использовать класс интеллектуальных указателей, и это позволит вам управлять памятью. В стандартной библиотеке C ++ есть умный указатель, называемый std :: auto_ptr, но он имеет странную семантику и не может использоваться в контейнерах C ++, таких как std :: vector. Если у вас установлены библиотеки Boost , то я бы предложил заменить ваши необработанные указатели на boost :: shared_ptr.

Answer 2

Я бы предложил, чтобы maxnum (и, возможно, imagelist) стал личным членом данных и реализовывал методы const getMaxnum() и setMaxnum(). Но я сомневаюсь, что это является причиной любого ошибки, как вы это описали.

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

Кроме того, вы можете создать отдельный объект класса Scene и передать данные типа изображения в виде указателя массива. И я не думаю, что вы можете объявить Image *imagelist[value];.

void Scene::copy(Image *sourceimagelist, int sourcemaxnum) {  
maxnum=sourcemaxnum;
imagelist=new Image[maxnum];
//...
    imagelist[i].xcoord = sourceimagelist[i].xcoord;
    imagelist[i].ycoord = sourceimagelist[i].ycoord;
//...
}
//...
Scene a,b;  
//...
b.Copy(a.imagelist,a.maxnum);