Reading each line from a text file, word by word and converting to int (Infinite loop or crashing?)

Question

I'm trying to read in this text file:

8 4 4 6 1
8 4 4 6 2
8 4 4 6 3
8 4 4 6 4
8 4 4 6 5
8 4 4 6 6
8 4 4 6 7
8 4 4 6 8
11 4 4 6 3
15 11 13
7 2 1 4 4 
9 4 3 9 9
8 2 1 5 4 
10 1 2 3 4 6 1
6 1 1 2 5 3 2
13 1 1 2 10 3 8 
11 2 11 10 7

And printing it exactly as shown to the console (to make sure I got every input).

However, for some reason my code crashes after reading in the first line. I can't even terminate the debugger.

Here's my code:

while(getline(inFile, buffer)){
    buffer2 = strdup(buffer.c_str());
    line = strtok(buffer2, " ");
    size = atoi(line);
    cout << size << " ";

    while(line!=NULL){
        line = strtok(NULL, " ");
        cout << line << " ";
    }

    cout << "~~~~~~~~~" << endl;
}

why are you calling all those `str_foo` when you just want to print the same you read? — 463035818_is_not_an_ai, Jan 29 '20 at 21:21
what is this line supposed to achieve `line = strtok(NULL, " ");` ? — 463035818_is_not_an_ai, Jan 29 '20 at 21:24
@formerlyknownas_463035818, that is a correct way to use `strtok`. See the example code at https://en.cppreference.com/w/cpp/string/byte/strtok. — R Sahu, Jan 29 '20 at 21:44
@formerlyknownas_463035818, why blame C only? The landscape of libraries -- standard or not -- is littered with sentinel values. — R Sahu, Jan 29 '20 at 21:52
After finding the last token in a line, the call to `strtok` will return a `NULL` pointer. `std::cout << line` won’t do anything good when that happens. — Pete Becker, Jan 29 '20 at 22:00
@RSahu, the question is, if you are using C++ why use `strtok()` for this? — anastaciu, Jan 29 '20 at 22:44

anastaciu · Answer 1 · 2020-01-30T08:59:33.400

If you are going to use C++ you should take advantage of that, use string streams:

#include <fstream>
#include <sstream>
#include <iostream>


using namespace std; //for sample purposes, should not be used

int main() {

    int temp, count = 0, sum = 0, total = 0;
    string buffer;
    ifstream myFile("in.txt");
    if (!myFile.is_open())
        cout << "No file" << endl;
    else{
        while(getline(myFile, buffer)){
            sum = 0;
            stringstream ss(buffer);
            while(ss >> temp){
                count++;         //number count            
                sum += temp;     //line sum
                cout << temp << " ";
            }
            total += sum;   //total sum
            cout << endl << "count: " << count <<  endl
                << "sum: " << sum << endl << "total: " << total << endl << endl;
        }
    myFile.close();
    }
    cout << "~~~~~~~~~" << endl;  
}

score 1 · Answer 2 · answered Jan 29 '20 at 22:30

You are leaking the memory allocated by strdup(). You need to call free() when you are done using buffer2.

But more importantly, strtok() returns NULL when there are no more tokens to return. But it is undefined behavior to pass a NULL char* pointer to operator<<. Your while loop is doing exactly that when it reaches the end of each line, so anything could happen, including crashing.

Try this instead:

while (getline(inFile, buffer)) {
    buffer2 = strdup(buffer.c_str());
    if (buffer2 != NULL) {
        line = strtok(buffer2, " ");
        while (line != NULL) {
            size = atoi(line);
            cout << size << " ";
            line = strtok(NULL, " ");
        }
        free(buffer2);
    }
    cout << "~~~~~~~~~" << endl;
}

That being said, why are you using strdup(), strtok(), and atoi() at all? You are writing C++ code, you should C++ semantics instead of C semantics. For example, you can use std::istringstream instead, eg:

while (getline(inFile, buffer)) {
    istringstream iss(buffer);
    while (iss >> size) {
        cout << size << " ";
    }
    cout << "~~~~~~~~~" << endl;
}

score 1 · Answer 3 · answered Jan 30 '20 at 14:56

As always, there are many possible solutions. I would like to show an additional one. This is using more modern C++ elements, mainly from the algorithm and iterator library.

So, what will we do?

First we read each line as a std::string in a simple for loop with std::getline. Then we will put the line again in a std::istringstream so that we can take advantage of C++ iterator: std::istream_iterator.

This iterator will iterate over the elements in the string and extract all integers. It is like calling the extractor operator ( >> ) for all elements in the line string.

We use the iterator in the so called range constructor of os a std::vector. This inplace created vector, will be added to the destiantion data. So, as a result, we will get vector of vector of int: A 2-dimensional vector.

For debug purposes, we copy each row of intes to std::cout.

Please note that we do really need only very few and very simple statements to fulfill the task.

Please check.

#include <iostream>
#include <string>
#include <algorithm>
#include <sstream>
#include <vector>
#include <iterator>

std::istringstream sourceFile{R"(8 4 4 6 1
8 4 4 6 2
8 4 4 6 3
8 4 4 6 4
8 4 4 6 5
8 4 4 6 6
8 4 4 6 7
8 4 4 6 8
11 4 4 6 3
15 11 13
7 2 1 4 4 
9 4 3 9 9
8 2 1 5 4 
10 1 2 3 4 6 1
6 1 1 2 5 3 2
13 1 1 2 10 3 8 
11 2 11 10 7)"};

int main()
{
    // Here we will store the resulting int values
    std::vector<std::vector<int>> data{};

    for (std::string line{}; std::getline(sourceFile, line); ) {

        // Split the line into integers and add to target array
        std::istringstream iss(line);
        data.emplace_back(std::vector<int>(std::istream_iterator<int>(iss), {}));
    }       

    // Now all data is in our vector of vector of int

    // Show read data on screen
    std::for_each(data.begin(), data.end(), [](const std::vector<int>& v){ 
        std::copy(v.begin(), v.end(), std::ostream_iterator<int>(std::cout, " ")); std::cout << "\n";});

    return 0;
}

Please note. I do not have files on SO. So I used a std::istringstream as input stream. You may of course exchange it with any other std::ftream

Reading each line from a text file, word by word and converting to int (Infinite loop or crashing?)

3 Answers3