Constructor being called when trying to use strtok

Question

I've written a custom string class called DSString, with a private member char* data. I'm trying to parse lines from a csv file, adding each word found in the line of a file to a std::set of DSString objects. I call getline to read in a line from the file into a char* buffer, then call strtok to get a word. However, the while loop below does not work; my debugger tells me that upon reaching that line of code, it calls my constructor in DSString.h with parameter NULL, then the program ends.

int main (int argc, char ** argv) {
    
    ifstream input;
    input.open(argv1);

    int reviewLinesCtr = 0;
    char line[16000];

    set <DSString>positiveWordSet;

    while (input.getline(line,16000)) {
        if (reviewLinesCtr == 10) break;

            //Push review to positive set
            DSString word = strtok(line, " ");
            while (word != nullptr) {         //Program ends abruptly here
                positiveWordSet.insert(word);
                word = strtok(nullptr, " ");
            }
        }
        reviewLinesCtr++;
    }

    return 0;
}

DSString only has private members char *data and int size. Here is the relevant code in DSString:

    DSString::DSString(const char* param) {  //This is the constructor being  passed, param = NULL, which crashes the program.
        data = new char[strlen(param) + 1];
        this->size = strlen(param) + 1;
        strcpy(data, param);
    }

    DSString& DSString::operator= (const char* source)  {
        if (data != nullptr) {
            delete[] data;
        }
        data = new char[strlen(source + 1)];
        size = strlen(source) + 1;
        strcpy(data, source);
        return *this;
    }

    bool DSString::operator!= (const DSString& rhs) const {
        if (strcmp(this->data, rhs.data) != 0) {
            return true;
        }
        else {
            return false;
        }
    } 

This is part of a project for college, so we are required to implement the custom DSString class. Otherwise I would not use it and instead use std::string. We also cannot use a char* anywhere in the program except for a buffer to read from the file. Why is this constructor being called and given NULL as a parameter when I try to start going through this while loop? And how can I fix it?

I apologize if this is a repeat question, I have been searching the internet for some time and have not found anything that helps explain what's going on here. Any help would be appreciated.

Answer 1

The cause of the crash is most likely outlined in eeorika's answer. No point repeating it here.

Why the constructor is called:

DSString word = strtok(line, " ");

Is Copy Initialization, so the constructor, not the assignment operator, is called.

Side notes:

There is another fatal error. Since strtok returns nullptr when there are no more tokens, DSString::DSString(const char* param) will be called if there are no tokens and

data = new char[strlen(param) + 1];

will invoke strlen on a null pointer. Kabooom.

Solution: Capture strtok's return value in a const char * variable and test it for nullptr. Only proceed with construction if there is a token.

Since the assignment requirements are that you cannot use a char * variable, confirm whether or not you can use a const char *. And if not, well that's an interesting (and really stupid) pickle.

The assignment operator leaks memory.

DSString& DSString::operator= (const char* source)  {
    delete[] data; // must release the object's current string
                   // before allocating a new one
    data = new char[strlen(source + 1)]; 
    size = strlen(source) + 1;
    strcpy(data, source);
    return *this;
}

Answer 2

DSString word = ....
while (word != nullptr) {         //Program ends abruptly here

Here, you compare a DSString object to nullptr. Let's look at the operator overaload:

bool DSString::operator!= (const DSString& rhs) const {

It accepts a DSString reference, not a pointer. However, your class is implicitly convertible from a pointer and thus indirectly from astd::nullptr_t because it has a converting constructor:

SString::DSString(const char* param) {  //This is the constructor being  passed, param = NULL, which crashes the program.

Since the constructor calls strlen(param) and strcpy(data, param);, passing null to that constructor has undefined behaviour.

To avoid accidentally calling this constrcutor with an implicit conversion, you should declare it explicit.

Passing null to the constructor is also a problem here: DSString word = strtok(line, " ");.

To fix the logic, you should use a pointer with strtok, and only create your string after you've checked that it's non-null:

        char* word = strtok(line, " ");

---

We also cannot use a char* anywhere in the program except for a buffer to read from the file.

Frankly, using strtok but not using char* is just silly requirement.

Seems like you already violate this requirement:

DSString only has private members char *data and ...

Regardless, if you need to follow such restriction, you need to change the constructor to behave correctly when null is passed into it. Specifically, it may not pass that pointer into functions which have undefined behaviour in such case.

And, if you set the member null, then you must also check whether the member is null before passing it to such functions mentioned above.