Can't add item to const char* vector? C++

Question

I have an issue where I need a vector of const char* but for some reason whenever I try adding something nothing happens. Here is the code sample in question.

std::vector<const char*> getArgsFromFile(char* arg) {
    std::ifstream argsFile(arg);
    std::vector<const char*> args;
    while (!argsFile.eof()) {
        std::string temp;
        argsFile >> temp;
        args.push_back(temp.c_str());
    }
    args.pop_back();
    return args;
}

The strange part is if I make this change

std::vector<const char*> getArgsFromFile(char* arg) {
    std::ifstream argsFile(arg);
    std::vector<const char*> args;
    while (!argsFile.eof()) {
        std::string temp;
        argsFile >> temp;
        const char* x = "x";
        args.push_back(x);
    }
    args.pop_back();
    return args;
}

It will add 'x' to the vector but I can't get the value of temp into the vector. Any thoughts? Help would be greatly appreciated. Thanks!

The return value of `c_str` goes out of scope when `temp` goes out of scope. `x` is working because it gets stuck in read-only storage in the binary so it never disappears. — Millie Smith, Mar 02 '18 at 23:53
You don't need a vector of char *, you need a vector of std::string. Also, your use of eof() is wrong - see https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong — , Mar 02 '18 at 23:53
I need a vector of char* its part of my homework we can't use a string vector — cainsr2, Mar 02 '18 at 23:56
Then you will need to call `new char[size]`, copy in the string, add the char array to the vector, and make sure to call `delete []` on everything you allocated at the end of the program. — Millie Smith, Mar 03 '18 at 00:01
It happens because you are using a *Temporary Object (rvalue)* (`const char* x = "x";`), check this link: http://thbecker.net/articles/rvalue_references/section_01.html — Jorge Omar Medra, Mar 03 '18 at 00:50
@MillieSmith: "*and make sure to call `delete []` on everything you allocated*" - if you must use `new char[]` instead of `std::string`, then you should use `std::vector>` instead of `std::vector`, and let `std::unique_ptr` handle the deallocation for you when the `vector` is cleared/destroyed. — Remy Lebeau, Mar 03 '18 at 02:27

Walter · Answer 1 · 2018-03-03T00:29:36.477

A const char* is not a string, but merely a pointer to some memory, usually holding some characters. Now std::string under the hood either holds a small region of memory (like char buff[32]) or, for larger strings, keeps a pointer to memory allocated on the heap. In either case, a pointer to the actual memory holding the data can be obtained via string::c_str(). But when the string goes out of scope that pointer no longer points to secured data and becomes dangling.

This is the reason why C++ introduced methods to avoid direct exposure and usage of raw pointers. Good C++ code avoid raw pointers like the plague. Your homework is for poor/bad C++ code (hopefully only to learn the problems that come with such raw pointers).

So, in order for the pointers held in your vector to persistently point to some characters (and not become dangling), they must point to persistent memory. The only guaranteed way to achieve that is to dynamically allocate the memory

while (!argsFile.eof()) {
    std::string temp;
    argsFile >> temp;
    char* buff = new char[temp.size()+1];          // allocate memory
    std::strncpy(buff,temp.c_str(),temp.size()+1); // copy data to memory
    args.push_back(buff);                          // store pointer in vector
}

but then the memory allocated in this way will be leaked, unless you de-allocate it as in

while(!args.empty()) {
    delete[] args.back();
    args.pop_back();
}

Note that this is extremely bad C++ code and not exception safe (if an exception occurs between allocation and de-allocation, the allocated memory is leaked). In C++ one would instead use std::vector<std::string> or perhaps std::vector<std::unique_ptr<const char[]> (if you cannot use std::string), both being exception safe.

The data managed by `std::string` is not necessarily stored in any heap. — Lightness Races in Orbit, Mar 03 '18 at 00:25
An `stdd::string` doesn't always hold a pointer - [small string optimization](https://stackoverflow.com/questions/10315041/meaning-of-acronym-sso-in-the-context-of-stdstring). — einpoklum, Mar 03 '18 at 00:25

einpoklum · Answer 2 · 2018-03-03T01:17:35.873

Use a standard-library-based implementation

Guideline SL.1 of the C++ coding guidelines says: "Use the standard library whenever possible" (and relevant). Why work so hard? People have already done most of the work for you...

So, using your function's declaration, you could just have:

std::vector<std::string> getArgsFromFile(char* arg) {
    using namespace std;
    ifstream argsFile(arg);
    vector<string> args;
    copy(istream_iterator<string>(argsFile),
          istream_iterator<string>(),
          back_inserter(args));
    return args;
}

and Bob's your uncle.

Still, @Walter's answer is very useful to read, so that you realize what's wrong with your use of char * for strings.

Can't add item to const char* vector? C++

2 Answers2

Use a standard-library-based implementation