How to pull text out of a .txt and store it into a dynamic 2d array?

Question

I need to pull text line by line out of my .txt file and store it into a dynamic array that has new space allocated every time I pull a new line out of the .txt file. My code seems to pull out the first line just fine and store it into the first pointers array, but on the second loop, it seems to reset all the pointers arrays which gives me memory allocation errors when I later try to access it. Why does this happen especially when I don't touch the pointers and their arrays after I store stuff into them?

char** temp = nullptr;
    char buffer[256];
    int index = 0;

    // Open File
    fstream myFile;
    myFile.open("pantry.txt", ios::in);
    if (myFile.is_open())
    {
        while (!myFile.eof())
        {
            myFile >> buffer; // Pull line out of txt.file

            temp = new char* [index + 1]; // Create new pointer

            temp[index] = new char[strlen(buffer)+1]; // Create char array pointed at by new pointer
#pragma warning(suppress : 4996) // Turns off complier warning
            strcpy(temp[index], buffer); //Copy buffer into new char array
            index++; // Increment our index counter int

            
        }

        for (int i = 0; i < index; i++)
        {
            cout << temp[i] << endl;
        }

If allocated and stored correctly I want it to just print out the txt file exactly. Instead, I get

Exception thrown at 0x7B9A08CC (ucrtbased.dll) in PE 12.4.exe: 0xC0000005: Access violation reading location 0xCDCDCDCD.

pantry.txt

Basil
Flat Leaf Parsely
Thyme
Sage
Cumin
Steak Seasoning
Mace
Garlic Powder

Unless you have a specific reason to do otherwise (such as doing homework that doesn't allow it) I'd strongly suggest that you create ` std::vector`, and use `std::getline` to read each line in and `std::push_back` to put each line into the `vector`. With that, the job becomes quite a bit easier. — Jerry Coffin, Oct 22 '21 at 21:15
What is `temp` supposed to represent? With vague variable names, you may not even notice that you are assigning something new to that variable over and over, always leaking (and losing) what it used to point to. — Drew Dormann, Oct 22 '21 at 21:18
Root cause of your issue (apart from not using modern c++ like @JerryCoffin says) is that you reallocate `temp` each loop, so the second loop you loose everything you stored in the first, etc. — Mike Vine, Oct 22 '21 at 21:19
Oh, I also just noticed that you're using `while (!myFile.eof())`. That's usually going to lead to a problem as well. You usually want something of the form `while (read_from_file_succeeded())` instead. — Jerry Coffin, Oct 22 '21 at 21:20
Expanding on above: [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) — user4581301, Oct 22 '21 at 21:28

score 0 · Accepted Answer · answered Oct 22 '21 at 22:05

There are multiple bugs in the shown code.

while (!myFile.eof())

This is always a bug that also must be fixed, in addition to the main problem with the shown code:

temp = new char* [index + 1];

To help you understand the problem with this line, it's helpful to remember The Golden Rule Of Computer Programming:

Your computer always does exactly what you tell it to do instead of what you want it to do.

According to the Golden Rule, the above line tells your computer, exactly: "new something, and assign it to temp".

And this is what your computer will do every time it executes this line. This line is executed once, on each iteration of this loop. The next time this loop runs, the previously newed temp will be replaced by another one, leaking everything that it pointed to before. Why should your computer do anything else, on this line? After all, this is exactly what you told your computer to do. And this is why you observed that this will "reset all the pointers arrays" on every iteration of the loop, resulting in "memory allocation errors".

In any case, this entire chunk of logic needs to be scrapped and rewritten from scratch, this time using the right logic. The simplest thing to do is to actually use the C++ library's std::vector and std::string objects, which will do all the memory allocation for you, correctly. Modern C++ code rarely needs to new anything, and will use C++ library's containers, instead.

It is possible that the goal of your assignment is to demonstrate proper use of low-level memory allocation and deallocation logic. In this case you will need to find some other way of doing this. Since you don't know the number of lines in advance, one approach will be to build a linked list, one line at a time, as each line gets read from the file. The final array, with all the character pointers gets allocated only after the entire file is read (and the number of lines is known), the pointers moved to the array, and the temporary linked list deleted. Or, perhaps implement a std::vector-like algorithm that progressively allocates a new pointer array, when it is full, copies over all the character pointers to a bigger array and then deletes the original one.

Of course, that's a lot of work. But unless the purpose of your assignment or task is to correctly implement low-level memory allocations and deallocation, why go through all the work and pain to do what std::vector and std::string already do, when you can simply use them, in just five or six lines of code, that will replace all of the above?

How to pull text out of a .txt and store it into a dynamic 2d array?

1 Answers1