[解決済み】C++ - 解放されるポインタが割り当てられていないエラー

質問

malloc: *** error for object 0x10ee008c0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

あるいは、すべてを印刷しようとすると、次のようになります。

Segmentation fault: 11

OOPの授業の宿題をやっているのですが、もう1時間くらい詰まっています。キーボード入力を十分に使ったところで、このようなエラーが発生しています。私は全くイライラしない人間なのですが、ここにきて非常にイライラしています。以下がそのファイルです。

これは本のクラスです。これはとてもしっかりしていると思います。でも、一応参考までに。

//--------------- BOOK.CPP ---------------
// The class definition for Book.
//

#include <iostream>
#include "book.h"

using namespace std;

Book::Book()
// 
{
  strcpy(title, " ");
  strcpy(author, " ");
  type = FICTION;
  price = 0;
}

void Book::Set(const char* t, const char* a, Genre g, double p)
{
  strcpy(title, t);
  strcpy(author, a);
  type = g;
  price = p;
}

const char* Book::GetTitle() const
{
  return title;
} 

const char* Book::GetAuthor() const
{
  return author;
}

double Book::GetPrice() const
{
  return price;
}

Genre Book::GetGenre() const
{
  return type;
}

void Book::Display() const
{
  int i;

  cout << GetTitle();
  for (i = strlen(title) + 1; i < 32; i++)
    cout << (' ');

  cout << GetAuthor();
  for (i = strlen(author) + 1; i < 22; i++)
    cout << (' ');

  switch (GetGenre())
  {
  case FICTION:
    cout << "Fiction   ";
    break;
  case MYSTERY: 
    cout << "Mystery   ";
    break;
  case SCIFI:
    cout << "SciFi     ";
    break;
  case COMPUTER:
    cout << "Computer  ";
    break;
  }

  cout << "$";
  if (GetPrice() < 1000)
    cout << " ";
  if (GetPrice() < 100)
    cout << " ";
  if (GetPrice() < 10)
    cout << " ";

  /* printf("%.2f", GetPrice());*/

  cout << '\n';
}

これは、配列とダイナミックアロケーションを扱うストアクラスです。これは入力コマンドを使わなくても、その関数を使うだけでうまく動作していました。

//--------------- STORE.CPP ---------------
// The class definition for Store.
//
#include <iostream>
#include <cstring>  // for strcmp
#include "store.h"

using namespace std;

Store::Store()
{
  maxSize = 5; 
  currentSize = 0; 
  bookList = new Book[maxSize];
}

Store::~Store()
// This destructor function for class Store
// deallocates the Store's list of Books
{
  delete [] bookList;
}

void Store::Insert(const char* t, const char* a, Genre g, double p)
// Insert a new entry into the direrctory.
{
  if (currentSize == maxSize)// If the directory is full, grow it.
    Grow();

  bookList[currentSize++].Set(t, a, g, p);
}

void Store::Sell(const char* t)
// Sell a book from the store.
{ 
  char name[31];
  strcpy(name, t);

  int thisEntry = FindBook(name);// Locate the name in the directory.

  if (thisEntry == -1)
    cout << *name << " not found in directory";
  else
    {
      cashRegister = cashRegister + bookList[thisEntry].GetPrice();
      // Shift each succeding element "down" one position in the
      // Entry array, thereby deleting the desired entry.
      for (int j = thisEntry + 1; j < currentSize; j++)
    bookList[j - 1] = bookList[j];

      currentSize--;// Decrement the current number of entries.
      cout << "Entry removed.\n";

      if (currentSize < maxSize - 5)// If the directory is too big, shrink it.
    Shrink();
    }
}

void Store::Find(const char* x) const
//  Display the Store's matches for a title or author.
{
  // Prompt the user for a name to be looked up

  char name[31];
  strcpy(name, x);

  int thisBook = FindBook(name);
  if (thisBook != -1)
    bookList[thisBook].Display();

  int thisAuthor = FindAuthor(name, true);

  if ((thisBook == -1) && (thisAuthor == -1))
    cout << name << " not found in current directory\n";
}

void Store::DisplayGenre(const Genre g) const
{
  double genrePrice = 0;
  int genreCount = 0;

  for (int i = 0; i < currentSize; i++)// Look at each entry.
  {  
    if (bookList[i].GetGenre() ==  g)
    {
      bookList[i].Display();
      genrePrice = genrePrice + bookList[i].GetPrice();
      genreCount++;
    }
  }
  cout << "Number of books in this genre: " << genreCount
       << "                    " << "Total:    $";
  if (genrePrice < 1000)
    cout << " ";
  if (genrePrice < 100)
    cout << " ";
  if (genrePrice < 10)
    cout << " ";

  printf("%.2f", genrePrice);
}

void Store::DisplayStore() const
{
  if (currentSize >= 1)
  {
    cout << "**Title**\t\t"
     << "**Author**\t"
     << "**Genre**\t"
     << "**Price**\n\n";

    for (int i = 0; i < currentSize; i++)
      bookList[i].Display();
  }
  else
    cout << "No books currently in inventory\n\n";

  cout << "Total Books = " << currentSize 
       << "\nMoney in the register = $";
  if (cashRegister < 1000)
    cout << " ";
  if (cashRegister < 100)
    cout << " ";
  if (cashRegister < 10)
    cout << " ";

  printf("%.2f", cashRegister);

  cout << '\n';
}

void Store::Sort(char type)
{
  Book temp;

  for(int i = 0; i <= currentSize; i++)
  {
    for (int j = i+1; j < currentSize; j++)
    {
      if (type == 'A')
      {
    if (strcmp(bookList[i].GetTitle(), bookList[j].GetTitle()) > 0)
    {
      temp = bookList[i];
      bookList[i] = bookList[j];
      bookList[j] = temp;
    }
      }
      if (type == 'T')
      {
    if (strcmp(bookList[i].GetAuthor(), bookList[j].GetAuthor()) > 0)
    {
      temp = bookList[i];
      bookList[i] = bookList[j];
      bookList[j] = temp;
    }
      }
    }
  }
}

void Store::SetCashRegister(double x)
// Set value of cash register
{
  cashRegister = x;
}

void Store::Grow()
// Double the size of the Store's bookList
// by creating a new, larger array of books
// and changing the store's pointer to refer to
// this new array.
{
  maxSize = currentSize + 5;// Determine a new size.

  cout << "** Array being resized to " << maxSize 
       << " allocated slots" << '\n';

  Book* newList = new Book[maxSize];// Allocate a new array.

  for (int i = 0; i < currentSize; i++)// Copy each entry into
    newList[i] = bookList[i];// the new array.

  delete [] bookList;// Remove the old array
  bookList = newList;// Point old name to new array.
}

void Store::Shrink()
// Divide the size of the Store's bookList in
// half by creating a new, smaller array of books
// and changing the store's pointer to refer to
// this new array.
{
  maxSize = maxSize - 5;// Determine a new size.

  cout << "** Array being resized to " << maxSize 
       << " allocated slots" << '\n';

  Book* newList = new Book[maxSize];// Allocate a new array.

  for (int i = 0; i < currentSize; i++)// Copy each entry into
    newList[i] = bookList[i];// the new array.

  delete [] bookList;// Remove the old array
  bookList = newList;// Point old name to new array.
}

int Store::FindBook(char* name) const
// Locate a name in the directory.  Returns the
// position of the entry list as an integer if found.
// and returns -1 if the entry is not found in the directory.
{
  for (int i = 0; i < currentSize; i++)// Look at each entry.
    if (strcmp(bookList[i].GetTitle(), name) == 0)
      return i;// If found, return position and exit.

  return -1;// Return -1 if never found.
}

int Store::FindAuthor(char* name, const bool print) const
// Locate a name in the directory.  Returns the
// position of the entry list as an integer if found.
// and returns -1 if the entry is not found in the directory.
{
  int returnValue;

  for (int i = 0; i < currentSize; i++)// Look at each entry.
    if (strcmp(bookList[i].GetAuthor(), name) == 0)
    {
      if (print == true)
    bookList[i].Display();
      returnValue = i;// If found, return position and exit.
    }
    else
      returnValue = -1;// Return -1 if never found.

  return returnValue;
}

さて、この人は少し仕事が必要な人です。空白の部分があるかもしれないので、それは無視してください。これは全ての入力を制御しているのですが、これが問題だと思います。

#include <iostream>
#include "store.h"

using namespace std;

void ShowMenu()
// Display the main program menu.
{
  cout << "\n\t\t*** BOOKSTORE MENU ***";
  cout << "\n\tA \tAdd a Book to Inventory";
  cout << "\n\tF \tFind a book from Inventory";
  cout << "\n\tS \tSell a book";
  cout << "\n\tD \tDisplay the inventory list";
  cout << "\n\tG \tGenre summary";
  cout << "\n\tO \tSort inventory list";
  cout << "\n\tM \tShow this Menu";
  cout << "\n\tX \teXit Program";
}

char GetAChar(const char* promptString)
// Prompt the user and get a single character,
// discarding the Return character.
// Used in GetCommand.
{
  char response;// the char to be returned

  cout << promptString;// Prompt the user
  cin >> response;// Get a char,
  response = toupper(response);// and convert it to uppercase
  cin.get();// Discard newline char from input.
  return response;
}

char Legal(char c)
// Determine if a particular character, c, corresponds
// to a legal menu command.  Returns 1 if legal, 0 if not.
// Used in GetCommand.
{
  return((c == 'A') || (c == 'F') || (c == 'S') || 
     (c == 'D') || (c == 'G') || (c == 'O') || 
     (c == 'M') || (c == 'X'));
}

char GetCommand()
// Prompts the user for a menu command until a legal 
// command character is entered.  Return the command character.
// Calls GetAChar, Legal, ShowMenu.
{
  char cmd = GetAChar("\n\n>");// Get a command character.

  while (!Legal(cmd))// As long as it's not a legal command,
    {// display menu and try again.
      cout << "\nIllegal command, please try again . . .";
      ShowMenu();
      cmd = GetAChar("\n\n>");
    }
  return cmd;
}

void Add(Store s)
{
  char aTitle[31];
  char aAuthor[21];
  Genre aGenre = FICTION;
  double aPrice = 10.00;

  cout << "Enter title: ";
  cin.getline(aTitle, 30);

  cout << "Enter author: ";
  cin.getline(aAuthor, 20);
  /*
  cout << aTitle << "  " << aAuthor << "\n";
  cout << aGenre << "  " << aPrice << '\n';
  */
  s.Insert(aTitle, aAuthor, aGenre, aPrice);

}

void Find()
{
}

void Sell()
{
}

void ViewGenre(Store s)
{
  char c;
  Genre result;

  do
    c = GetAChar("Enter Genre - (F)iction, (M)ystery, (S)ci-Fi, or (C)omputer: ");
  while ((c != 'F') && (c != 'M') && (c != 'S') && (c != 'C'));

  switch (result)
    {
    case 'F': s.DisplayGenre(FICTION);    break;
    case 'M': s.DisplayGenre(MYSTERY);    break;
    case 'S': s.DisplayGenre(SCIFI);      break;
    case 'C': s.DisplayGenre(COMPUTER);   break;
    }

}

void Sort(Store s)
{
  char c;
  Genre result;

  do
    c = GetAChar("Enter Genre - (F)iction, (M)ystery, (S)ci-Fi, or (C)omputer: ");
  while ((c != 'A') && (c != 'T'));

  s.Sort(c);
}

void Intro(Store s)
{
  double amount;

  cout << "*** Welcome to Bookstore Inventory Manager ***\n"
       << "Please input the starting money in the cash register: ";
  /*  cin >> amount;

      s.SetCashRegister(amount);*/
}

int main()
{
  Store mainStore;// Create and initialize a Store.

  Intro(mainStore);//Display intro & set Cash Regsiter

  ShowMenu();// Display the menu.

  /*mainStore.Insert("A Clockwork Orange", "Anthony Burgess", SCIFI, 30.25);
    mainStore.Insert("X-Factor", "Anthony Burgess", SCIFI, 30.25);*/

  char command;// menu command entered by user
   do
     {
       command = GetCommand();// Retrieve a command.
       switch (command)
     {
     case 'A': Add(mainStore);             break;
     case 'F': Find();                     break;
     case 'S': Sell();                     break;
     case 'D': mainStore.DisplayStore();   break;
     case 'G': ViewGenre(mainStore);       break;
     case 'O': Sort(mainStore);            break;
     case 'M': ShowMenu();                 break;
     case 'X':                             break;
     }
     } while ((command != 'X'));

   return 0;
}

どうか、どんなことでもご協力をお願いします。 ありがとうございます。

解決方法は？

C++のエキサイティングな世界へようこそ!

簡単な答え：Storeを値として渡している。メニュー関数はすべてStore&かStore*を受け取るようにしましょう。

Storeを値として渡す場合、暗黙のうちにコピーが行われます（したがって、mainStore変数が実際に変更されることはありません）。関数から戻ると、Store::~Storeが呼び出され、コピーされたデータがクリーンアップされます。これにより、実際のポインタ値を変更することなく、mainStore.bookListが解放されます。

さらにメニューを操作すると、メモリが破壊され、何度も二重解放が行われます。

HINT: もしあなたがLinuxを使っているなら、プログラムをvalgrindで実行すれば、ほとんどの簡単なメモリエラーを指摘してくれるでしょう。