C++ class with char pointers returning garbage

Question

I created a class "Entry" to handle Dictionary entries, but in my main(), I create the Entry() and try to cout the char typed public members, but I get garbage. When I look at the Watch list in debugger, I see the values being set, but as soon as I access the values, there is garbage. Can anyone elaborate on what I might be missing?

#include  <iostream>

using  namespace  std;

class Entry
{
    public:
            Entry(const char *line);
            char *Word;
            char *Definition;
};

Entry::Entry(const char *line)
{
    char tmp[100];
    strcpy(tmp, line);

    Word = strtok(tmp, ",") + '\0';
    Definition = strtok(0,",") + '\0';
}

int  main()
{
    Entry *e = new Entry("drink,What you need after a long day's work");
    cout << "Word: " << e->Word << endl;
    cout << "Def: " << e->Definition << endl;
    cout << endl;

    delete e;
    e = 0;

    return  0;
}

Answer 1

strtok() returns a pointer into its input string. You are passing it a buffer on the stack, which is no longer valid after Entry::Entry returns.

Answer 2

Word and Definition both point into tmp, which has gone out of scope and so contains garbage.

Answer 3

tmp should be a class member, to hold the string that Word and Definition point to. Otherwise, as the other answers mentioned, tmp will get out of scope once the constructor returns. Another thing is that you shouldn't add that char to the pointers. I assume you wanted to put a terminator in the string, but things don't work that way in C/C++. In effect you are adding the ASCII value (zero) to the pointer as an offset, which does nothing. If you want a terminator you need to change the characters pointed by the pointer, not change the pointer itself. But strtok already puts terminators at the end of each token found, anyway - there's no need for that in this case.

So, my suggestion:

#include  <iostream>

using  namespace  std;

class Entry
{
    public:
            Entry(const char *line);
            char *Word;
            char *Definition;

    private:
            char buffer[100];


};

Entry::Entry(const char *line)
{
    strncpy(buffer, line, sizeof buffer);
    buffer[sizeof buffer - 1] = '\0';

    Word = strtok(buffer, ",");
    Definition = strtok(0,",");
}

int  main()
{
    Entry *e = new Entry("drink,What you need after a long day's work");
    cout << "Word: " << e->Word << endl;
    cout << "Def: " << e->Definition << endl;
    cout << endl;

    delete e;
    e = 0;

    return  0;
}

I changed the name from tmp to buffer, since it's not a temporary value anymore. I also used strncpy to prevent a buffer overflow. The line buffer[sizeof buffer - 1] = '\0'; is there because if line is bigger than buffer, it won't have the terminator after the call.

Answer 4

Actually, if it's within the constraints of your assignment, I'd recommend abandoning char* and strtok() in favor of string, istringstream, and getline():

#include <string>
#include <sstream>
#include <iostream>

using namespace std;

class Entry
{
  public:
    Entry(const string& line);
    string Word;
    string Definition;
};

Entry::Entry(const string& line)
{
  istringstream iss(line);
  getline(iss, Word, ',');
  getline(iss, Definition, ',');
}

int main()
{
  Entry e = Entry("drink,What comes between \"eat\" and \"be merry\"");
  cout << "Word: " << e.Word << endl;
  cout << "Def: " << e.Definition << endl;
  cout << endl;

  return  0;
}