1

I'm having an issue when running the code below. Every time I set the while loop to reach the .eof() it returns a std::bad_alloc

inFile.open(fileName, std::ios::in | std::ios::binary);

        if (inFile.is_open())
        {
            while (!inFile.eof())
            {
                read(inFile, readIn);
                vecMenu.push_back(readIn);
                menu.push_back(readIn);
                //count++;
            }

            std::cout << "File was loaded succesfully..." << std::endl;

            inFile.close();
        }

It runs fine if I set a predetermined number of iterations, but fails when I use the EOF funtion. Here's the code for the read function:

void read(std::fstream& file, std::string& str)
{
    if (file.is_open())
    {
        unsigned len;
        char *buf = nullptr;

        file.read(reinterpret_cast<char *>(&len), sizeof(unsigned));

        buf = new char[len + 1];

        file.read(buf, len);

        buf[len] = '\0';

        str = buf;

        std::cout << "Test: " << str << std::endl;

        delete[] buf;
    }
    else
    {
        std::cout << "File was not accessible" << std::endl;
    }
}

Any help you can provide is greatly appreciated. NOTE: I failed to mention that vecMenu is of type std::vector and menu is of type std::list

Akatosh
  • 13
  • 4
  • 3
    Please see this post: [Why is iostream::eof inside a loop condition considered wrong?](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – Rakete1111 Jan 02 '17 at 02:55
  • 1
    Also, calling the allocator for every single line you read in is slowing down your program. Better to use a non-local `std::vector` and issue a `resize()` call than issuing `new / delete` calls each and every time. – PaulMcKenzie Jan 02 '17 at 03:03
  • @RSahu Why? That's exactly the issue. – Baum mit Augen Jan 02 '17 at 03:10
  • Add sanity check(s) to the value of `len` read from the file. – Richard Critten Jan 02 '17 at 03:10
  • @BaummitAugen, that's just of one of the problems in the OP's code. – R Sahu Jan 02 '17 at 03:12
  • @RSahu It's the single key issue here, so I feel like the cv was the right thing to do. (Just explaining myself, not trying to argue.) – Baum mit Augen Jan 02 '17 at 03:25
  • @BaummitAugen, my gut feeling is that the `std::bad_alloc` arose from not checking whether `ifstream::read` was successful. Constructing a `std::string` from an array of `char`s that was not initialized properly is not good. – R Sahu Jan 02 '17 at 03:30
  • @BaummitAugen, I hope you understand why I re-opened the question. – R Sahu Jan 02 '17 at 03:33
  • @PaulMcKenzie Thank you, I will definitely modify it to use a vector. Thank you for the suggestion. I just started programming, and this is good good practice. – Akatosh Jan 02 '17 at 05:25
  • @RSahu If you would be so kind as to point out what else is wrong with my code. I would like to work on fixing all the problems. This will help me a great deal. Thank you – Akatosh Jan 02 '17 at 05:27
  • @Akatosh, the only other problem I saw from the posted code was the return value of your `read` function, which was not conducive to being used in the conditional of a `while` loop. My answer already addresses that. – R Sahu Jan 02 '17 at 05:36

1 Answers1

1

The main problems I see are:

  1. You are using while (!inFile.eof()) to end the loop. See Why is iostream::eof inside a loop condition considered wrong?.

  2. You are not checking whether calls to ifstream::read succeeded before using the variables that were read into.

I suggest:

  1. Changing your version of read to return a reference to ifstream. It should return the ifstream it takes as input. That makes it possible to use the call to read in the conditional of a loop.

  2. Checking whether calls to ifstream::read succeed before using them.

  3. Putting the call to read in the conditional of the while statement.

std::ifstream& read(std::fstream& file, std::string& str)
{
   if (file.is_open())
   {
      unsigned len;
      char *buf = nullptr;

      if !(file.read(reinterpret_cast<char *>(&len), sizeof(unsigned)))
      {
         return file;
      }

      buf = new char[len + 1];

      if ( !file.read(buf, len) )
      {
         delete [] buf;
         return file;
      }

      buf[len] = '\0';

      str = buf;

      std::cout << "Test: " << str << std::endl;

      delete[] buf;
   }
   else
   {
      std::cout << "File was not accessible" << std::endl;
   }

   return file;
}

and

inFile.open(fileName, std::ios::in | std::ios::binary);

if (inFile.is_open())
{
   std::cout << "File was loaded succesfully..." << std::endl;

   while (read(inFile, readIn))
   {
      vecMenu.push_back(readIn);
      menu.push_back(readIn);
      //count++;
   }

   inFile.close();
}
Community
  • 1
  • 1
R Sahu
  • 204,454
  • 14
  • 159
  • 270