2009-09-19 7 views
0

У меня есть домашнее задание, которое я почти закончил.Повреждение кучи, возможные утечки памяти, C++

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

quack::quack(int capacity) : backPtr(NULL), frontPtr(NULL) 
{ 
    items = new item[capacity]; 
    backPtr = new item; 
    frontPtr = new item; 
    midPtr = new item; 
    current = new item; 

    maxSize = capacity; 
    back = maxSize-1; 
    count = 0; 
    top  = -1; 
} 

quack::~quack(void) 
{ 
    delete frontPtr; 
    delete backPtr; 
    delete current; 
    delete midPtr; 
    delete [] items; //Heap Corruption Debug Error at the end of program. 

    items = NULL; 
    maxSize = 0; 
    back  = 0; 
} 

bool quack::pushFront(const int n) 
{  
    int i = 0; 

    if (count == maxSize) // Then we cant add to it n e more. 
    { 
     throw runtime_error("Stack is Full");// Full Stack 
     return false; 
    } 
     backPtr->n = items[back-1].n; 
     while (i < count) // Loop less than however many we counted. 
     { 
      if (i == top+1) 
      { 
       current->n = items[top+1].n; 
       items[top+1].n = backPtr->n; 
      } 

      midPtr->n = items[++i].n; 
      items[i].n = current->n; 

      if (i != back-1) 
      { 
       current->n = items[++i].n; 
       items[i].n = midPtr->n; 
      } 
     } 
     ++count; 
     items[top+1].n = n; 
     return true;  
} 

bool quack::pushBack(const int n) 
{ 
    items[count].n = n; 
    count++; 
    return true; 
} 

bool quack::popFront(int& n) 
{ 
    n = items[top+1].n; 
    for (int i = 0; i < count; i++) 
    { 
     items[i] = items[i+1]; 
    } 
    count--; // Remove top element. 
    return true; 
} 

bool quack::popBack(int& n) 
{ 

    n = items[--count].n; 
    return true; 
} 

void quack::rotate(int r) 
{ 
    int i = 0; 

    while (r > 0) // rotate postively. 
    { 
     frontPtr->n = items[top+1].n; 
     for (int i = 0; i < back; i++) 
     { 
      items[i] = items[i+1];     
     } 
     items[back-1].n = frontPtr->n; 
     r--; 
    } 

    while (r < 0) // rotate negatively. 
    { 
     if (i == top+1) 
     { 
      backPtr->n = items[back-1].n; 
      current->n = items[top+1].n; 
      items[top+1].n = backPtr->n; 
     } 
      midPtr->n = items[++i].n; 
      items[i].n = current->n; 

      if (i == back-1) 
      { 
       items[back-1].n = current->n; 
       i = 0; 
       r++; continue; 
      } 
     else 
     { 
      current->n = items[++i].n; 
      items[i].n = midPtr->n; 
      if (i == back-1) 
      { 
       i = 0; 
       r++; continue; 
      } 
     } 
    } 
} 

void quack::reverse(void) 
{ 
    int j = 0; // Variable declaration/initialization. 

    frontPtr->n = items[top+1].n; 
    backPtr->n = items[back-1].n; 

    for (int i = 0; i < count/2; i++) 
    { 
     items[j].n = items[i].n; 
     items[i].n = items[ count - i-1 ].n; 
     items[ count - i-1 ].n = items->n; 
    } 

    items[top+1].n = backPtr->n; 
    items[back-1].n = frontPtr->n; 
} 

int quack::itemCount(void) 
{ 
    return count; 
} 

ostream& operator<<(ostream& out, quack& q) 
{ 
    if (q.count == 0) // No elements have been counted. 
    out << endl << "quack: empty" << endl; 
    else 
    { 
     out << endl << "quack: "; 
     for (int i = 0; i < q.count; i++) 
     { 
      if (i < q.count-1) 
       out << q.items[i].n << ", "; 
      else out << q.items[i].n; 
     } 
     out << endl << endl; 
    } 
    return out; 
} 

и заголовочный файл:

#include <ostream> 

using namespace std; 

class quack 
{ 
public: 
    quack(int capacity); 
    ~quack(void); 
    bool pushFront(const int n); // Push an item onto the front. 
    bool pushBack(const int n);  // Push an item onto the back. 
    bool popFront(int& n);   // Pop an item off the front. 
    bool popBack(int& n);   // Pop an item off the back. 
    void rotate(int r);    // "rotate" the stored items (see note below). 
    void reverse(void);    // Reverse the order of the stored items. 
    int itemCount(void);   // Return the current number of stored items. 

private: 
    int maxSize; // is for the size of the item stack 
    int back; // is for the back or "bottom" of the stack 
    int count; // to count the items added to the stack 
    int top; 

    struct item      // Definition of each item stored by the quack. 
    { 
     int  n; 
    }; 

    item  *items;    // Pointer to storage for the circular array. 
    item  *backPtr; 
    item  *frontPtr; 
    item  *midPtr; 
    item  *current; 

public: 
    friend ostream& operator<<(ostream& out, quack& q); 
}; 
+0

Я читал форматирование снова и снова * 5, и я просто действительно не понимаю. Может быть, если кто-то скажет мне, я, наконец, пойму. – user40120

+0

Можете ли вы переформатировать, вставив весь свой код? –

+1

Как отредактировать это правильно, когда его «как я хочу, чтобы оно было» в окне редактирования, но другое, когда его результат. – user40120

ответ

5

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

Конструктор

items = new item[capacity]; 
backPtr = new item; 
frontPtr = new item; 
midPtr = new item; 
current = new item; 

Это не имеет смысла. Ваши указатели на лицевую/заднюю/среднюю/текущую позиции фактически не указывают на один из ваших элементов. Вероятно, вы хотите иметь frontPtr = items+0 и backPtr = items + capacity-1 (или наоборот). Не уверен, что для dequeue нужен midPtr или текущий.

[Изменить: Кажется, элемент struct item { int n }, и вы просто копируете n вокруг. И у вас есть индекс задний и верхний индекс ...]

деструктор

delete frontPtr; 
delete backPtr; 
delete current; 
delete midPtr; 
delete [] items; //Heap Corruption Debug Error at the end of program. 

С передней/задней/и т.д.. должен указывать внутри элементов, вы дважды освобождаете некоторые из этих элементов. Это может быть крах коррупции в куче. [Edit: или нет, учитывая странное копирование]

items  = NULL; 
maxSize = 0; 
back   = 0; 

Это кажется довольно глупо (объект не собирается быть больше, кого это волнует?) ...

Err, какие

Хорошо, нормальный путь простой Dequeue будет работать, чтобы иметь массив элементов:

items  -> 1 2 3 4 5 6 7 8 9 … 
front_ptr -----------/     /|\ 
back_ptr --------------------------------+ 

, а затем вы бы иметь указатель (frontPtr), который указывает на первое место б в массиве и другой указатель (backPtr) что указывает на последнее используемое место. Таким образом, pop_front бы сделать что-то вроде этого:

if (frontPtr <= backPtr) { 
    return *frontPtr++ 
} else { 
    // tried to pop from empty dequeue, handle error 
} 

Это было возвращение 3, и заранее front_ptr, чтобы указать на 4. pop_back будет аналогично (но с тестом обратной, и - вместо ++) ,

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

+0

, используя этот способ реализации метода rotate, становится проще, просто добавляя (или вычитая, в зависимости от направления) индексы (или указатели, независимо от того, что вы используете) – Vargas

+0

@Vargas: Поскольку OP не заботится о производительности, Повернуть очень просто: push_back (pop_front()), повторяется, как всегда. – derobert

0

Запустить его с помощью valgrind.

+2

Ему не нужен какой-то инструмент, ему нужно понять, что он там делает. – karx11erx

+0

Это будет точный _output_ инструмента. – Zed

+0

Нет, это не так, потому что valgrind не объяснил ему, какие указатели и их различные виды использования/приложения. – karx11erx

2

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

При выделении

items = new item[capacity] 

вы фактически были указывающего указатель «пункты» в первый элемент массива «элемента» выделяется в куче:

items -> {item}{item}{item}..{item} // capacity 

Ваши другие указатели должны затем укажите один и тот же массив и не должны впоследствии использоваться для удаления того, на что они указывают, вместо этого вы просто удаляете «элементы»:

delete [] items 

Массивы и указатели взаимозаменяемы.

items + 1 - это то же самое, что и & элементов [1] * (items + 1) - это то же самое, что и элементы [1].

Это делает «текущий» точка ко второму элементу в массиве:

current = items + 1 

Это не работает, потому что «текущий» является указателем и элементы [1] является объектом элемент массива, не является адрес:

current = items[1] 

Это работает, однако:

current = &items[1] 
+0

+1 для ответа только на вопрос – Vargas

1

Ваша главная проблема в том, чтобы понять, что backptr, фр ontptr, midptr и current имеют другую цель, чем элементы. Технически все они указывают на некоторое местоположение памяти, но семантически элементы являются якорем для вашего контейнера данных (массива), в то время как целью других указателей является управление этими данными (бухгалтерский учет).

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

В результате вы должны установить backptr, frontptr, midptr и current в NULL в c'tor и d'tor и использовать только новые и удалять с элементами.

Btw, я не знаю, разрешена ли ваша задача, но использование связанного списка вместо массива может облегчить вашу жизнь в управлении списком (как, например, использование типа int для backptr, frontptr, midptr и current) ,

1

Возможны две причины сбоя при удалении массива items.

  1. Класс не имеет специального конструктора копирования или оператора присваивания. Это важно, потому что встроенные будут бесполезны. Если вы когда-либо передали quack по значению в качестве параметра или возвращали один из функции или копировали quack в переменную, у вас должно быть два quack s, указывающие на тот же массив items. Когда второй разрушает, он будет пытаться сделать delete массив во второй раз, и, как заметил Джон Леннон в своей знаменитой песне об удалении объектов кучи, «Нет, нет, не второй раз».

  2. Вы много записываете в массив items в своем коде. Если вы пишете за конец (или до начала) массива, это будет обнаружено при его удалении, потому что в этот момент реализация кучи обнаружит, что вы уничтожили некоторую управляющую информацию, необходимую для освобождения блока памяти ,Возможно, у вас есть ошибка в этом коде, как это делают большинство людей, когда они начинают пытаться это сделать.

В C++ существует простой способ обнаружить это - не используйте необработанный массив. Используйте класс, который обертывает массив и выполняет проверку границ при каждом доступе к нему. Вы можете получить это с помощью std::vector и использовать функцию at вместо оператора [] (который не проверен на границах).

+0

В вашем (2) написании перед массивом элементов будет, скорее всего, возникнет кучевое повреждение. Большинство реализаций, похоже, помещают информацию о выделенном блоке памяти непосредственно перед блоком. На самом деле, если бы я мог, я бы отдал это больше. –