Проблема с деструктором класса

1. "Перестаёт работать" - понятие довольно широкое. Что именно происходит?
2. Не совсем понятно, почему в конструкторе не инициализируется first->str (оно же last->str), и почему len = 0, когда ты в список тут же добавляешь один элемент.
3. В add надо использовать strcpy вместо strcat.
4. Комментарии бы не помешали - до меня, например, не доходит, почему это всё вообще работает.

Большое спасибо за отклик, вот ответы на Ваши вопросы: 1. При втором вызове функции TEST получаем core dumped. Если же закомментировать деструктор - все работает. Но в разрабатываемой мной программе снова получаю core dumped. 2. Если я правильно понимаю, то конструктор срабатывает только при инициализации, т. е. на строке, в которой я объявляю массив тест: dynarr test; Поэтому нам не важно на что указывает first->str, т. к. на месте first будет первый добавленный элемент 3. Спасибо, я чисто машинально написал strcat :( 4. Вот комментарии:

class dynarr { struct elem { char *str; elem *next; }; elem *first; elem *last; int len; public: dynarr(); ~dynarr(); void add(const char *p); int size(){return len;}; char* get(const int num); char* cat(const int num=0); }; dynarr::dynarr(){ // создаем "пустой" первый элемент. Он будет перезаписан // первым добавленным элементом first= new elem; first->next=first; last=first; len=0; } dynarr::~dynarr(){ // мне кажется, есть очень глупая ошибка где-то здесь // объявляем указатель на текущий элемент elem *n; n=first; // цикл с первого до последнего элемента while (n!=last){ delete [] n->str; delete n; n=n->next; } // а теперь удаляем последний элемент delete [] last->str; delete last;

} // функция добавления элемента в массив void dynarr::add(const char *p){ // выделяем память под новый элемент массива и заполняем этот элемент elem *n; n = new elem; n->str=new char [strlen(p)+1]; strcpy(n->str,p); // т. к. элемент добавляется в конец - его next будет указывать на // него же n->next=n; // last указывает на последний элемент. Указываем last на новый // элемент last->next=n; last=n; // если у нас еще нет элементов - делаем новый элемент первым if (len==0) first=n; len++; }

// функция получения элемента массива (указателя на строку) по // заданному номеру элемента char* dynarr::get(const int num){ int number=num; // проверка на выход за границы массива if (number < 0 || number > len) number=len; // объявляем указатель на элемент массива и указываем его на первый // элемент elem *cur=first; int i=0; // двигаем указатель по массиву до тех пор, пока не дойдем до // нужного элемента while(i<number){ cur=cur->next; i++; } return cur->str; }

// эта функция возвращает указатель на строку, которая является // конкатенацией первых num элементов массива. Если num не указывается // то всех элементов массива char* dynarr::cat(const int num=0){ int number=num; // проверка на выход за границы массива if(number <= 0 || number >len) number=len; elem *cur=first; // подсчитываем длину всех элементов, которые будет соединять воедино // sz=1, т. к. строку будет завершать 0 int sz=1; for (int n=0; n<number; n++){ sz+=strlen(cur->str); cur=cur->next; } // объявляем указатель на строку и выделяем память под строку char *p; p=new char [sz]; *p='\0'; cur=first; // осуществляем конкатенацию элементов в одну строку for (int n=0; n<number; n++){ strcat(p,cur->str); cur=cur->next; } return p; }

Большое спасибо за отклик, вот ответы на Ваши вопросы:
1. При втором вызове функции TEST получаем core dumped. Если же
закомментировать деструктор - все работает. Но в разрабатываемой мной
программе снова получаю core dumped.
2. Если я правильно понимаю, то конструктор срабатывает только при
инициализации, т. е. на строке, в которой я объявляю массив тест:
dynarr test; Поэтому нам не важно на что указывает first->str, т. к.
на месте first будет первый добавленный элемент
3. Спасибо, я чисто машинально написал strcat :(
4. Вот комментарии:

class dynarr {
struct elem {
char *str;
elem *next;
};
elem *first;
elem *last;
int len;
public:
dynarr();
~dynarr();
void add(const char *p);
int size(){return len;};
char* get(const int num);
char* cat(const int num=0);
};
dynarr::dynarr(){
// создаем "пустой" первый элемент. Он будет перезаписан
// первым добавленным элементом
first= new elem;
first->next=first;
last=first;
len=0;
}
dynarr::~dynarr(){
// мне кажется, есть очень глупая ошибка где-то здесь
// объявляем указатель на текущий элемент
elem *n;
n=first;
// цикл с первого до последнего элемента
while (n!=last){
delete [] n->str;
delete n;
n=n->next;
}
// а теперь удаляем последний элемент
delete [] last->str;
delete last;

}
// функция добавления элемента в массив
void dynarr::add(const char *p){
// выделяем память под новый элемент массива и заполняем этот элемент
elem *n;
n = new elem;
n->str=new char [strlen(p)+1];
strcpy(n->str,p);
// т. к. элемент добавляется в конец - его next будет указывать на
// него же
n->next=n;
// last указывает на последний элемент. Указываем last на новый
// элемент
last->next=n;
last=n;
// если у нас еще нет элементов - делаем новый элемент первым
if (len==0) first=n;
len++;
}

// функция получения элемента массива (указателя на строку) по
// заданному номеру элемента
char* dynarr::get(const int num){
int number=num;
// проверка на выход за границы массива
if (number < 0 || number > len) number=len;
// объявляем указатель на элемент массива и указываем его на первый
// элемент
elem *cur=first;
int i=0;
// двигаем указатель по массиву до тех пор, пока не дойдем до
// нужного элемента
while(i<number){
cur=cur->next;
i++;
}
return cur->str;
}

// эта функция возвращает указатель на строку, которая является
// конкатенацией первых num элементов массива. Если num не указывается
// то всех элементов массива
char* dynarr::cat(const int num=0){
int number=num;
// проверка на выход за границы массива
if(number <= 0 || number >len) number=len;
elem *cur=first;
// подсчитываем длину всех элементов, которые будет соединять воедино
// sz=1, т. к. строку будет завершать 0
int sz=1;
for (int n=0; n<number; n++){
sz+=strlen(cur->str);
cur=cur->next;
}
// объявляем указатель на строку и выделяем память под строку
char *p;
p=new char [sz];
*p='\0';
cur=first;
// осуществляем конкатенацию элементов в одну строку
for (int n=0; n<number; n++){
strcat(p,cur->str);
cur=cur->next;
}
return p;
}

Смотрите на второй ответ, первый с неотформатированными строками, ошибся при постинге :(

Весь код не просматривал -- лень, но так, навскидку:

[skip]
// цикл с первого до последнего элемента
while (n!=last){
delete [] n->str;
delete n;
n=n->next;
^^^ -- n уже удалено в предыдущей строке.
}
[skip]

Смотри на свой дестрактор, который написан с грубой ошибкой - как-то обращение к удаленной памяти. Я уж не говорю про ОЧЕНЬ странный метод реализации односвязного списка, а так же явный пролет в дизайне elem, т.к. delete elem_var; при правильно дихайне класса должен удалаять все свои внутренности сам.
dynarr::~dynarr()
{ 
elem *n; 
n=first; 
// цикл с первого до последнего элемента 
while(n!=last)
{ 
delete [] n->str; 
delete n; 
n=n->next; // ERROR: n does not exist anymore!
} 

// а теперь удаляем последний элемент 
delete [] last->str; 
delete last; 
} 

Соотвественно функция должна быть реализована как:
dynarr::~dynarr()
{ 
elem *n;
while( first != last )
{
n = first;
first = first->next;
delete [] n->str;
delete n;
}

// а теперь удаляем последний элемент 
delete [] last->str; 
delete last; 
} 

Тот же дестрактор но при правильном дизайне (правильный односвязный список и правильный elem):
dynarr::~dynarr()
{
elem *eTmp;
while( !first )
{
eTmp = first;
first = first->next;
delete eTmp;
}
}

Короче говоря, при ответе на вопрос "почему глючет деструктор" я отвечу
коротко.

Нельзя удалять содержимое строки elem::str;
ведь ты же не знаешь, откуда взялись эти данные в общем случае.
Да даже зная это - ты ведь засовываешь в свой класс автоматические
переменные:

test.add("12345");

ты удаляешь все это оператором delete[]! Это, безусловно, грубейшая ошибка. Скажу более. Код надо тестировать постепенно, удалив сначала всякие обращения к методам. Оставь только создание переменной класса dynarr (деструктор будет вызываться) - и все равно ты получишь коредамп.
Почему? А как у тебя в конструкторе инициализируется elem::str?
Да никак. :-( Ну и привет. Удаляем delete[] неизвестно что :-((

> Нельзя удалять содержимое строки elem::str;
> ведь ты же не знаешь, откуда взялись эти данные в общем случае.
> Да даже зная это - ты ведь засовываешь в свой класс автоматические
> переменные:
> test.add("12345");
> ты удаляешь все это оператором delete[]! Это, безусловно,
> грубейшая ошибка.

Глядя на исходник видим оператор new и strcpy(). Так что мимо тазика :))

void dynarr::add(const char *p){
elem *n;
n = new elem;
n->str=new char [strlen(p)+1];
strcpy(n->str,p);
[skip]
}

> Почему? А как у тебя в конструкторе инициализируется elem::str?
> Да никак. :-( Ну и привет. Удаляем delete[] неизвестно что :-((
Что-то мне подсказывает, что elem::str будет содержать 0, так как
для elem вызовется сгенеренный дефолтный конструктор
(хотя в этом могу ошибаться).

2asd
Да, есть мой косяк: в код add я не заглянул, каюсь... Так что с этим ОК.
Но, поверь мне, никакого нуля в elem::str не записывается.
Это ява так себя ведет, а С++ нет.

2 JekLove (*) (2002-03-11 20:50:24.0):

А ты бы попробовал -- для elem автоматически генерится конструктор и str и next зануляются. Я, для интереса проверил -- все Ok. gcc-2.96-98, gcc-3.0.1-3

2asd (*) (2002-03-12 10:07:45.0):
> для elem автоматически генерится конструктор и str и next зануляются
Специально стандарты посмотрел - нет там такого!
В ISO/IEC 14882:1998(E) 5.3.4 New 15 написано, что, если опустить
инициализатор у new, то создаваемые объекты подобного типа имеют
неопределенное значение.

2 Die-Hard:

Да, конечно, если ты создаешь с помощью new объекты интегрального типа, то они не зануляются. А в нашем случае разве не будет вызван дефолтный конструктор для elem? Как он, кстати, будет выглядеть?

Кстати, в продолжение темы:

#include <iostream>
#include <cstdio>

int
main(void) {
char *s;
int *i;

for ( int j = 0; j < 1000; j++ ) {
s = new char;
fprintf(stderr, "s=0x%x (%d)\n", (unsigned) s, *s);
i = new int;
fprintf(stderr, "i=0x%x (%d)\n", (unsigned) i, *i);
}
}

Как бы всегда нули в скобках получаются...
Это недокументированная фича gcc что-ли или я торможу под вечер?

Похоже все-таки торможу :)) В последнем сообщении new не при чем, это у меня malloc так работает.

asd (*) (2002-03-12 17:18:44.0):
> разве не будет вызван дефолтный конструктор для elem?
Там (в Стандартах) очень не просто разобраться -словно специально написано так,
чтобы запутать. Но, как я понял (на днях посмотрю аккуратнее),
для структур и юнионов дефолтный конструктор не строится.

2asd (*) (2002-03-12 18:32:15.0):
> В последнем сообщении new не при чем, это у меня malloc так работает.
Дело твое, конечно, но я б подумал, прежде чем такой malloc себе
заводить. Ни малейшего смысла не вижу. И программы твои абсолютно непортабельными
получаются. И "стиль" страдает.

Вот, прочитал все что гнал здесь вчера под вечер... Явно голова уже не работала :((

Резюме:

1. Die-Hard абсолютно прав насчет неопределенного состояния элементов elem после вызова конструктора для dynarr.

2. Дефолтный конструктор, про который пишет Страуструп (я не смог придумать, где я еще мог про него услышать), в данном случае выглядит похожим на new char[sizeof(elem)] (это следует из разглядывания ассемблерного кода). gcc-2.96-98, gcc-3.0.4-1.

3. malloc, который отдает память заполненную нулями не есть мой специальный, он из glibc-2.2.4. Видимо это связано с его реализацией (mmap от /dev/zero, сорсов glibc под рукой нет, точно утверждать не могу). Я, кстати, никогда на такое поведение malloc'а не закладывался и вчера сам удивлялся.

4. Случайно отмечено, что new является простым враппером для malloc, а слова Страуструпа о том, что ``new выделяет несколько больше памяти (обычно на слово) для хранения размера элемента'' не выполняются (по крайней мере для gcc-2.95.2). Единственное, что new действительно делает, это преобразует new(0) --> new(1) и пытается вызвать хандлеры в случае, если malloc вернет 0. delete[] от delete не отличается совсем и вызов delete(0) абсолютно не страшен, так как стоит проверка. Естесственно на это закладываться ни в коем случае нельзя, так как в стандартах этого нет.

Огромное всем спасибо за замечания и предложения по решению проблемы. Мой "ОЧЕНЬ странный метод реализации односвязного списка" заработал после того, как я:

1. Добавил инициализацию first->str=0 в деструкторе (точно нужно, я убедился на практическом примере: дефолтный конструктор при объявлении переменной класса dynarr в какой-нибудь подпрограмме (функции) генерит не 0, а случайное значение)
2. Изменил цикл уничтожения элементов в деструкторе (спасибо asd и Ogr-у)