CODEWITHSUNDEEP
c++arrayschar
ontherocks
ontherocks

Reputation: 1999

Populate char* array using loop

I have a legacy function that has an input parameter of type const char* array

myfunc(const char* names[]);

I have a set std::set<std::string> mynames;. The set has already been populated. I have to pass mynames to myfunc

Using range based loop, I have

const char* names[];
int i = 0;
for (const auto &name : mynames)
{
    //allocate memory to names here
    names[i] = name.c_str();
    ++i;
}

//call myfunc
myfunc(names);

Is there a better way to do this?

Upvotes: 0

Views: 1023

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 596497

You are not allocating any memory for the names[] array. You need to do something more like this:

const char** names = new const char*[mynames.size()]; // +1 if needed

size_t i = 0;
for (const auto &name : mynames)
{
    names[i++] = name.c_str();
}
/* if needed:
names[i] = nullptr;
*/

/* alternatively
std::transform(mynames.begin(), mynames.end(), names,
    [](const std::string &name){ return name.c_str(); }
);
// if needed :
// names[mynames.size()] = nullptr;
*/

myfunc(names);
delete[] names;

Which can then be simplified further by using std::vector instead of new[], eg:

std::vector<const char*> names;
names.reserve(mynames.size()); // +1 if needed
for (const auto &name : mynames)
{
    names.push_back(name.c_str());
}
/* if needed:
names.push_back(nullptr);
*/

/* alternatively:
std::vector<const char*> names(mynames.size()); // +1 if needed
std::transform(mynames.begin(), mynames.end(), names.begin(),
    [](const std::string &name){ return name.c_str(); }
);
// if needed:
// names.back() = nullptr;
*/

myfunc(names.data());

Upvotes: 2

Kenny Ostrom
Kenny Ostrom

Reputation: 5871

If you can rewrite this function, then do so. However, there are plenty of longstanding third party libraries like this, and you don't want to try to mess with them.

You are going to have to allocate a temporary data structure that provides the data in the format that function expects. You are going to have to figure out how long it has to exist. I'll show a sample here that assumes that you can skip allocating the individual strings, and just allocate one array for their pointers (as you have done).

You didn't allocate the space for the pointers, and you didn't add a nullptr at the end. Notice that the function takes a raw array, so it can't know how many names there are. It will almost certainly expect a NULL to signal the last name.

#include <iostream>
#include <string>
#include <set>

// This demonstrates how a legacy function like this might work
int myfunc(const char* names[]) {
    if (names) {
        int index = 0;
        const char* name = names[index];

        // Notice that this function cannot know how many names there are.
        // The function documentation will almost certainly tell you 
        // to add a NULL as the last entry.
        while (name) {
            std::cout << name << std::endl;
            name = names[++index];
        }
    }
    return 0;
}

int main()
{
    const char * legacy [] = {
        "one",
        "two",
        "three",
        nullptr,
    };

    std::cout << "legacy" << std::endl;
    myfunc(legacy);


    std::set<std::string> modern { "one", "two", "three" };

    // this temp structure is valid as long as it is 
    // allocated, and original set doesn't change
    // although you probably should allocate and clean up
    // a copy of each string within this temp array.
    const char ** temp = new const char* [modern.size()+1];
    int index = 0;
    for (const auto &name : modern) {
        temp[index++] = name.data();
    }
    temp[index] = nullptr;

    // if myfunc just uses them and exits, you're okay
    // if it remembers any of the pointers for later, that would be unfortunate
    std::cout << "using temp" << std::endl;
    myfunc(temp);

    // delete what we allocated. The pointers inside are owned by the set, so we don't care about that
    delete [] temp;
    return  0;
}

Upvotes: 2

Related Questions