Нужна помощь в устранении утечки памяти - PullRequest
1 голос
/ 10 октября 2019

Я пишу систему управления запасами с использованием динамических массивов с использованием объектов. Стратегия для этого - НЕ использовать векторы, а выделять динамический массив, который увеличивается на 1 каждый раз, когда клиент должен добавить в инвентарь. Я получаю ошибку «ошибка сегментации», поэтому считаю, что это утечка памяти.

Я пытался переписать его, чтобы он соответствовал адресам, но безуспешно. Я думаю, что функция buildItem создает временный объект, который уничтожается при завершении функции. Я не знаю, как это исправить.

#include <iomanip>
#include <iostream>
#include <string>
using namespace std;

class InventoryObject {
private:
  int itemNumber;
  string description;
  int qty;
  float price;

public:
  int getItemNum() { return itemNumber; }
  string getDescription() { return description; }
  int getQty() { return qty; }
  float getPrice() { return price; }

  void storeInfo(int p, string d, int q, float pr);
  void showValues(InventoryObject &item);
};

// Function Implementation
void InventoryObject::storeInfo(int p, string d, int q, float pr) {
  itemNumber = p;
  description = d;
  qty = q;
  price = pr;
}

void InventoryObject::showValues(InventoryObject &item) {
  cout << fixed << showpoint << setprecision(2) << endl;
  cout << "Part Number  : " << item.getItemNum() << endl;
  cout << "Description  : " << item.getDescription() << endl;
  cout << "Quantity:    : " << item.getQty() << endl;
  cout << "Price        : " << item.getPrice() << endl << endl;
}

// Function Prototypes for Client Program
InventoryObject buildItem();
void drawMenu();
void showValues(InventoryObject &);
void printInventory(int size);

int main() {
  int size = 1;
  int choice;
  bool quit = false;
  InventoryObject part;
  InventoryObject *iArray = new InventoryObject[size];
  drawMenu();
  cin >> choice;
  while (quit == false) {
    if (choice == 1) {
      InventoryObject item;
      item = buildItem();
      iArray[size] = item;
    }
    if (choice == 2) {
      iArray[size].showValues(iArray[size]);
    }
    if (choice == 3) {
      quit = true;
    }
  }

  return 0;
}

// This function accepts the data from the client and creates a new
// InventoryObject object. The object is then supposed to be added to the
// dynamic array.
InventoryObject buildItem() {
  InventoryObject *tempObject = new InventoryObject;
  int itemNum;
  string description;
  int qty;
  float price;

  cout << "Enter data for the item you want to enter:\n\n";
  cout << "Item Number: \n";
  cin >> itemNum;
  cout << "Description: \n";
  cin.get();
  getline(cin, description);
  cout << "Quantity: \n";
  cin >> qty;
  cout << "Unit price: \n";
  cin >> price;

  tempObject->storeInfo(itemNum, description, qty, price);
  return *tempObject;
}

void drawMenu() {
  cout << "1. Add Inventory\n";
  cout << "2. Display Inventory\n";
  cout << "3. Quit Program\n";
}

Я ожидаю, что объект будет создан и помещен в динамический массив. Затем перерисовать меню и оттуда взаимодействовать с клиентом.

Ответы [ 4 ]

2 голосов
/ 10 октября 2019

Проблема main заключается в том, что вы пишете за пределами области памяти, выделенной для вашего массива (iArray).

В частности, эта строка кода:

iArray[size] = item;

На самом деле должно быть:

iArray[size - 1] = item;

Вышеуказанное не приводит к утечке памяти, но что-то еще, что вы делаете в своей программе:

Ваша функция buildItemделает это, когда возвращает значение указателя без предварительного удаления указателя.

Чтобы исправить это, измените

InventoryObject *tempObject = new InventoryObject;

на

InventoryObject tempObject;

И, наконец, запомнитеудалить iArray до return 0; в основной

delete[] iArray
0 голосов
/ 10 октября 2019

Я пытался переписать его, чтобы он соответствовал адресам, но не повезло. Я думаю, что функция buildItem создает временный объект, который уничтожается при завершении функции.

Ваша интуиция наполовину верна. С этой функцией что-то не так, но она не имеет ничего общего с временными. Это больше о том, что вы выделяете объект, который вы никогда не уничтожаете.

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

Дело в том, что суть проблемы заключается в том, чтоесли вы используете new для выделения чего-либо, вы должны использовать delete для его освобождения, чего вы здесь не делаете.

InventoryObject buildItem() {
  InventoryObject *tempObject = new InventoryObject;
  //...
  return *tempObject;
}

Вы просто возвращаете объектзначение, не удаляя его в первую очередь. Вы даже не возвращаете его по ссылке, что было бы лучше (но все же, вероятно, плохо: снова, см. мой вопрос ). Вы выделяете объект и никогда не удаляете его. Это утечка.

У вас есть два решения: либо вернуть указатель напрямую (если вам действительно нужен указатель), например так:

InventoryObject* buildItem() {
  InventoryObject *tempObject = new InventoryObject;
  //...
  return tempObject;
}

Или вы можете просто вернуть объект:значение как это:

InventoryObject buildItem() {
  InventoryObject tempObject;
  //...
  return tempObject;
}

Учитывая пример кода, который вы показали, это моя рекомендация из двух.


Примечание: я должен упомянуть, что если по какой-то причине выВам нужно вернуть указатель и иметь в игре какой-то шаблон фабричного метода с полиморфизмом, у вас также есть опция smart pointers . Именно это я и сделал, основываясь на предложениях других людей из другой ветки. Я могу лично рекомендовать это. По сути, ваш код стал бы примерно таким:

std::unique_ptr<InventoryObject> buildItem() {
  std::unique_ptr<InventoryObject> tempObject = new InventoryObject; // C++11 way, I think
                  // C++14 and above has a function called make_unique you could use instead.
  //...
  return tempObject;
}

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


P. (P.) S. Этот ответ касается только вашей предполагаемой утечки памяти. Тем не менее, здесь есть и другие полезные ответы, на которые вам следует обратить внимание. Проблемы, описанные в других ответах, могут усугубить вашу проблему и могут действительно сбить вас с толку (на самом деле, могут все же сбить вас с толку и сейчас), если вы их не исправите, поэтому я настоятельно рекомендую воспользоваться их советом.

0 голосов
/ 10 октября 2019

Даниэль, вот мои наблюдения:

  1. Функция buildItem должна возвращать указатель InventoryObject я имею в виду: InventoryObject*. Это потому, что вы возвращаете указатель памяти, которая была зарезервирована на момент ее создания (InventoryObject *tempObject = new InventoryObject;), например:
InventoryObject* buildItem() {
  InventoryObject *tempObject = new InventoryObject;

  //...

  tempObject->storeInfo(itemNum, description, qty, price);
  return *tempObject;
}

имейте в виду, что ваш основноймассив (InventoryObject *iArray) содержит массив указателей.

Основная проблема, которую я вижу. В main вы используете блок if, где есть локальная переменная , называемая item (InventoryObject item). Позвольте мне кое-что объяснить: каждый экземпляр класса, такой как вы InventoryObject item, будет уничтожен, освободите свою память, если его контекст закончен.

Итак, в данном случае проблема в том, что вы приводили указатель в момент возврата из функции buildItem (return *tempObject) и сохраняли его значение в локальной переменной InventoryObject item, который должен быть освобожден, когда поток покидает if, контекст локальной переменной.

Измените локальную переменную InventoryObject item для обработки указателя: InventoryObject* item

    if (choice == 1) {
      InventoryObject* item;
      item = buildItem();
      iArray[size] = item;
    }

Рекомендую прочитать главы 7. Указатели, массивы и ссылки и 17 Создание, очистка, копирование и перемещение из книги Язык программирования C ++ из Бьярн Страуструп .

0 голосов
/ 10 октября 2019
  1. Создайте свою программу с символом отладки (флаг -g)
  2. Запустите limit-c без ограничений
  3. Запустите вашу программу до ее сбоя
  4. Запустите ядро ​​GDB, гдеимя вашей программы
  5. В климатическом типе gdb bt.

Теперь у вас должна быть обратная информация о том, где происходит сбой вашей программы

...