0

I am trying to store a value from a

vector<vector<string>> data;

into a const char* variable - declared in the .h file - in this way:

heightmapName = data[0][1].c_str();

When I debug the program I notice that the variable heightmapName return this

heightmapName 0xcdcdcdcd <Error reading characters of string.>  const char *

However, if I declare a new const char* and initialize it like this:

    const char* what = data[0][1].c_str();
    heightmapName = data[0][1].c_str();

what variable store the data just fine, while heightmapNamedoesn't.

This is the function:

void Configuration::fileParser(string fileName)
{
    vector<vector<string>> data;
    string line;
    string delimiter = " ";
    ifstream ss(fileName);
    if (ss)
    {
        while (getline(ss, line))
        {
            vector<string> dataLine;
            string token = line.substr(0, line.find(delimiter));
            string value = line.substr(line.find(delimiter) +1);
            dataLine.push_back(token);
            dataLine.push_back(value);
            data.push_back(dataLine);
        }
        ss.close();
    }
    //storeData(data);
    const char* ahah = data[0][1].c_str();
    heightmapName =    data[0][1].c_str();
}

Why is this happening? and how can I solve this?

ps. I am using visual studio 2017

Swagging
  • 21
  • 1
  • 6
  • 0xcdcdcdcd .. Looks like uninitialized heap memory: https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations – drescherjm Aug 10 '17 at 02:03
  • 2
    Please post a [mcve], No one knows how, when, or where `data` is initialized, used, filled with data, etc. – PaulMcKenzie Aug 10 '17 at 02:06
  • I edited the question with the code of the function. – Swagging Aug 10 '17 at 02:08
  • `heightmapName` is undefined. It has no type. This code is not complete and probably won't compile. Is it a field within the `Configuration` class? – Steve Aug 10 '17 at 02:09
  • What is `heightmapName` declared as? If it is a `char*`, why is it not a `std::string` instead? Also, you are not checking if `data` is empty (because the file could not be opened, or the file is empty) before accessing `data[0]`. The vector's `operator[]` does not do any bounds checking (its `at()` method does) – Remy Lebeau Aug 10 '17 at 02:11
  • It is a const char*. It's not here because declared in the .h file – Swagging Aug 10 '17 at 02:12
  • 1
    Also, you posted a member function of a class. If that is not a static member function, and if that `Configuration` object is not valid, no operations that you will do on it will be valid. You need to show us a [mcve], not a snippet from a class. – PaulMcKenzie Aug 10 '17 at 02:13
  • @Steve `const char*` is not a const pointer, like you are thinking. It is a *non-const* pointer to a const `char`. So it can be reassigned to point at a different address, you just can't modify data through it. A const pointer would be like `char* const` (or `const char* const`) instead. Placement of `const` matters. – Remy Lebeau Aug 10 '17 at 02:15
  • There are multiple things wrong with this code, with the most prominent one being a lack of error checking. 1) What if that line doesn't have a space? You will then wind up using `std::string::npos` as the start of the `value` string. 2) What if there are no items in `data` when all is said and done? – PaulMcKenzie Aug 10 '17 at 02:18
  • @PaulMcKenzie I know, but as the .txt file I am reading from is written by me and contains only one line, I was trying to focus on this problem first. – Swagging Aug 10 '17 at 02:21
  • Reducing to a minimal complete example would probably show the problem...it has been suggested twice. If I try assigning the value of `c_str` to a `const char *` it works fine. – Matt Aug 10 '17 at 02:28
  • @matt, there is enough info there to see the problem. It's never going to work as-is. – Arafangion Aug 10 '17 at 02:29
  • @Swagging The moral of this whole story is stop using `const char *` when `std::string` is what you want. If a function requires a `const char *`, use the `c_str()` function *directly* instead of storing the return value in your own `const char *`. – PaulMcKenzie Aug 10 '17 at 02:31
  • @PaulMcKenzie: Even that won't work unless you know that the result of c_str() won't be used after the string has changed or become released. – Arafangion Aug 10 '17 at 02:32
  • @Matt: Swagging did actually produce a gist which "worked", but had the undefined behaviour of showing the string instead of the access violation, as he was using data after the lifetime had ended. (That gist was removed, for some reason) – Arafangion Aug 10 '17 at 02:35
  • @Arafangion -- I understand, but my point is that storing the return of `c_str()` in your own `const char *` member variable (at least in this case) is not necessary and just leads to problems such as this (Example: the `std::string` changes or is destroyed). If `heightMapName` were `std::string`, probably none of these issues would have occurred. – PaulMcKenzie Aug 10 '17 at 02:42
  • @PaulMcKenzie: Sure, that's a reasonable style guide. – Arafangion Aug 10 '17 at 02:44

1 Answers1

2

Regardless of the problem or implementation, assuming that the type of heightmapName is indeed const char *, that's not going to work.

The lifetime of the data is bound by the fileParser lifetime. See What is std::string::c_str() lifetime?

Therefore, at the end of that function, the data pointed to by data[0][1].c_str() will become invalid.

Consider copying the data if required. Or make heightmapName an std::string.

(Additional tips: If it's a pointer, consider applying the Rule Of Five: How to actually implement the rule of five? - another reason to avoid manual memory management)

In general, I avoid raw pointers in C++ classes by using smart pointers, or structures (such as std::string) that manages memory for me, this means I do not need to worry about the rule of 3 or rule of 5 as I won't need to manually manage those resources.

update: You mentioned that it "works" for you in a (now removed) gist.

Accessing memory like that after the lifetime has ended is undefined. One behaviour could very well be that it magically "works". Most likely, that memory just hasn't been overwritten yet.

Arafangion
  • 11,517
  • 1
  • 40
  • 72