strtok never gets NULL value, and ignores code after the while loop

Question

I am writing a program right now that tokenizes multiple strings from a text file.

I have a while loop that continues to run until the end of the file. And nested in that loop, I have another while loop using strtok() to tokenize strings.

But, in my nested while loop with strtok(), I test with an if statement to see if I get NULL at the end of a string, but I never receive the NULL value. After the nested loop, it seems to break out of both loops, and also skips code outside of the main while loop, and ends my program with return 0;.

#include<iostream>
#include<fstream>
#include<string.h>
#include<iomanip>

using namespace std;

int main(int argc, char* argv[])
{
    //variable declarations
    ifstream inputfile;
    ofstream outputfile;
    char line[100];
    char* ptr;

    //check how many arguments in command line
    cout << "argc: " << argc << endl << endl;

    //check if there is input and output command line argument
    if (argc < 3)
    {
        cout << "missing input and output files";
    }
    cout << argv[1] << endl;
    cout << argv[2] << endl << endl;
    //cout <<"Null: " << NULL << endl;

    //opens input file from command line argument
    inputfile.open(argv[1]);

    //opens output file from command line argument
    outputfile.open(argv[2]);

    //check if input file is open
    if (!inputfile)
    {
        cout << "file not open"<< endl;
    }


    //check for input
    while (!inputfile.eof())
    {

        //inputfile = input, getline, line = output, 100 = delimiter
        //grabs the line.
        inputfile.getline(line, 100);

        //check for line
        cout << line << endl;

        //grabs first word
        ptr = strtok(line, " ");
        cout << ptr << endl;

        //loops until null
        while(ptr!=NULL)
        {
            //grabs other words until end of line
            ptr = strtok(NULL, " ");
            cout << ptr << endl;


            if (ptr==NULL)
            {
                cout << "hit NULL" << endl;
            }

        }


        cout << "afterloop" << endl;

    }

    cout << "end";

    return 0;
}

Might be unrelated, but [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) Easy to fix, and no point to having two bugs in the code to untangle. — user4581301, Feb 12 '22 at 01:26
If allowed, get rid of the character arrays and `strtok` and use `std::string` and a `std::istringstream`. There's a great example [in option two of this answer](https://stackoverflow.com/a/7868998/4581301). — user4581301, Feb 12 '22 at 01:28
Good on you for checking `argc` before using `argv`. Far too many people get this wrong. — user4581301, Feb 12 '22 at 01:33
@user4581301 except that the code doesn't exit on an unacceptable `argc` value, so validating its value is useless. — Remy Lebeau, Feb 12 '22 at 01:36
Thanks, this project also requires the use of strtok instead of std::string. — Victor Li, Feb 12 '22 at 01:36
A nitpick here, `endl` also flushes every time you use it. Let the output buffer do its job by using `'\n'` when you need a newline. — chris, Feb 12 '22 at 01:41

chris · Answer 1 · 2022-02-12T01:47:34.500

In:

cout << ptr << endl;

//loops until null
while(ptr!=NULL)

And again in:

cout << ptr << endl;

if (ptr==NULL)

You're printing ptr before checking if it's null. It's undefined behaviour to hand operator<< a null character pointer. Rather, it assumes that the pointer points to a valid null-terminated string.

You cannot rely on any behaviour that comes out of this. The program is free to crash, pretend to work, or anywhere in between. The compiler is free to assume this doesn't happen, which can lead to it marking code as unreachable, among other optimizations, meaning you can see some unintuitive results.

While I would suggest using a better string-splitting approach, you can fix what you have by rearranging your code a bit:

//grabs first word
ptr = strtok(line, " ");

//loops until null
while(ptr!=NULL)
{
    // +++ We know we have a word, now we can print it.
    cout << ptr << endl;

    //grabs other words until end of line
    ptr = strtok(NULL, " ");
    // +++ We can remove the print from here because it will be printed on the next iteration if we have another word.

    if (ptr==NULL)
    {
        cout << "hit NULL" << endl;
    } 
}

Remy Lebeau · Answer 2 · 2022-02-12T04:50:51.453

As @chris's answer stated, you are invoking undefined behavior by passing NULL pointers to operator<<.

And, you should NOT use inputfile.eof() in a while loop. Loop on inputfile.getline() instead and let it break the loop when it can't read another line.

You are also not validating things very well in general, and not exiting the program when pre-conditions are not satisfactory.

Try something more like this instead:

#include <iostream>
#include <fstream>
#include <iomanip>
#include <string.h>

using namespace std;

int main(int argc, char* argv[])
{
    //variable declarations
    ifstream inputfile;
    ofstream outputfile;
    char line[100];
    char* ptr;

    //check how many arguments in command line
    cout << "argc: " << argc << endl << endl;

    //check if there is input and output command line argument
    if (argc < 3)
    {
        cout << "missing input and output files";
        return 0;
    }

    cout << argv[1] << endl;
    cout << argv[2] << endl << endl;
    //cout << "Null: " << NULL << endl;

    //opens input file from command line argument
    inputfile.open(argv[1]);

    //opens output file from command line argument
    outputfile.open(argv[2]);

    //check if input file is open
    if (!inputfile.is_open())
    {
        cout << "input file not open" << endl;
        return 0;
    }

    //check if output file is open
    if (!outputfile.is_open())
    {
        cout << "output file not open" << endl;
        return 0;
    }

    //check for input
    while (inputfile.getline(line, 100))
    {
        //check for line
        cout << line << endl;

        //grabs first word
        ptr = strtok(line, " ");
        if (ptr != NULL)
        {
            do
            {
                cout << ptr << endl;

                //grabs other words until end of line
                ptr = strtok(NULL, " ");
            }
            while (ptr != NULL);

            cout << "hit NULL" << endl;
        }

        cout << "afterloop" << endl;
    }

    cout << "end";

    return 0;
}

Thanks a lot for the tips and help! This is my 2nd CS class and it is quite challenging — Victor Li, Feb 12 '22 at 02:25

strtok never gets NULL value, and ignores code after the while loop

2 Answers2