Better way of building dynamic struct with char*?

Question

I want to use a C-style API from C++ which takes a variable number of structs with char* members like this:

typedef struct _FOO
{
    const char * name;
    const char * value;
} FOO;

void ApiFunc(FOO const* args, unsigned count);

To fill the parameters, I need to loop over some other data and create FOO entries on the fly. What would be the most elegant way to do that?

The following approach seems simple at first, but does not work (since the string instances go out of scope and are destroyed before the call to ApiFunc()):

// Approach A: this does *not* work
std::vector<FOO> args;

for (...)
{
   string name = ...   // something which gets
   string value = ...  // calculated in the loop
   args.push_back( FOO{name.c_str(), value.c_str()} );
}

ApiFunc( args.data(), args.size() );

Putting the string objects in a vector (to prevent them from being destroyed) doesn't work either - as the strings are copied when put into the vector, and the original ones are still destroyed:

// Approach B: this also does *not* work
std::vector<string> strings;
for (...)
{
   string name = ...   // something which gets
   string value = ...  // calculated in the loop
   strings.push_back( name );
   strings.push_back( value );
   args.push_back( FOO{name.c_str(), value.c_str()} );
}

ApiFunc( args.data(), args.size() );

I can prevent that by creating the string objects on the heap and using auto_ptr to keep track of them, but is there a better way?

// Approach C: this does work
std::vector<auto_ptr<string>> strings;
for (...)
{
   string* name = new ...   
   string* value = new  ... 
   strings.push_back( auto_ptr<string>(name) );
   strings.push_back( value );
   args.push_back( FOO{name.c_str(), value.c_str()} );
}

ApiFunc( args.data(), args.size() );

While approach C. seems to work, I find it rather unobvious / hard to understand. Any suggestions how I could improve it?

On an unrelated note, don't use symbols with leading underscore followed by an upper-case letter, [those are reserved in all scopes](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier) for "the implementation" (compiler and standard library). — Some programmer dude, Jan 12 '16 at 14:01
Why can't you either a) make those `char*` to `char[SOME_SIZE]`, so you could just copy over the strings and be done with it, or b) if the maximum size of a string can't be done, allocated the `name` \ `value` dynamically? — Algirdas Preidžius, Jan 12 '16 at 14:01
You [cannot use `auto_ptr` in a `vector`](http://stackoverflow.com/a/2643757/1600898). — user4815162342, Jan 12 '16 at 14:14
@AlgirdasPreidžius: He's interfacing with a C API from C++ code that uses `std::string`. There's no reason to assume he has any control over the C API. — Nicol Bolas, Jan 12 '16 at 14:15
Many thanks to all those who sent an answer or comment, I'm really learning a lot here and I'm very grateful for all the good input! — chrisv, Jan 13 '16 at 10:32

Martin Bonner supports Monica · Accepted Answer · 2016-01-12T15:35:05.903

I am pretty sure that std::vector<auto_ptr<T>> is not permitted by the standard. Use unique_ptr instead. Alternatively, build the strings in the vector, and then build args from that.

std::vector<std::pair<std::string, std::string>> strings;
for (...)
{
     const std::string name = ...;
     const std::string value = ...;
     strings.push_back( std::make_pair( name, value ) );
}
// Note: This loop must be separate so that 'strings' will not reallocate and potentially
// invalidate the pointers returned by elements in it.
for (const auto& pair : strings)
{
    args.push_back( FOO{pair.first.c_str(), pair.second.c_str()} );
}

ApiFunc( args.data(), args.size() );

Nicol Bolas · Answer 2 · 2016-01-12T14:17:35.643

2

Since you're interfacing with a C API, you're going to have to do things the C API way. That is, allocate heap buffers and deallocate them afterwards. This is actually a prime example of the utility of unique_ptr<T[]>:

//Helper function
std::unique_ptr<char[]> cpp_strdup(const std::string &str)
{
    std::unique_ptr<char[]> ret = new char[str.size() + 1];
    //Must copy the NUL terminator too.
    std::copy(str.data(), str.data() + str.size() + 1, ret.get());
    return ret;
}

//In your function:
std::vector<unique_ptr<char[]>> strings;

for (...)
{
   string name = ...   // something which gets
   strings.push_back(cpp_strdup(name));

   string value = ...  // calculated in the loop
   strings.push_back(cpp_strdup(value));

   args.push_back( FOO{strings[strings.size() - 2].get(), strings[strings.size() - 1].get()} );
}

By using unique_ptr instead of std::string in the strings vector, you neatly avoid reallocating the strings themselves when strings undergoes reallocation.

edited Jan 12 '16 at 14:17

answered Jan 12 '16 at 14:13

Nicol Bolas

449,505
63
781
982

Can you see a reason why my solution won't work (without involving manual heap allocation)? – Martin Bonner supports Monica Jan 12 '16 at 14:14
@MartinBonner: Nobody said it wouldn't work. But it is kind of expensive. My version of the `strings` vector never reallocates the contained strings. – Nicol Bolas Jan 12 '16 at 14:17
I would want to run benchmarks before I was convinced about performance. Your version doesn't reallocate because it has effectively thrown away SSO - and it can be a significant optimization. – Martin Bonner supports Monica Jan 12 '16 at 14:26

ciamej · Answer 3 · 2016-01-12T14:09:55.700

1

You can use the strdup function.

string name = ...   // something which gets
string value = ...  // calculated in the loop
args.push_back( FOO{strdup(name.c_str()), strdup(value.c_str())} );

Afterwards, you'll have to delete the strings.

free(foo.name);
free(foo.value);

edited Jan 12 '16 at 14:09

answered Jan 12 '16 at 14:05

ciamej

6,918
2
29
39

1

Isn't that a POSIX only function – NathanOliver Jan 12 '16 at 14:07
@NicolBolas yes, of course! I'll add this to my answer. – ciamej Jan 12 '16 at 14:08
@NathanOliver I think so. Still, you can reimplement it yourself. It's basically 4 lines of code. – ciamej Jan 12 '16 at 14:09

score 0 · Answer 4 · answered Jan 12 '16 at 14:00

0

You could use the strings stored in the vector:

   string name = ...   // something which gets
   string value = ...  // calculated in the loop
   strings.push_back( name );
   strings.push_back( value );
   args.push_back( FOO{strings[strings.size()-2].c_str(), strings[strings.size()-1].c_str()} );

answered Jan 12 '16 at 14:00

Georg

324
1
7

That will break the moment that `strings` is reallocated, which will copy/move all of the strings, thus invalidating the pointers to them (small-string optimization). – Nicol Bolas Jan 12 '16 at 14:01
2

So you need to `reserve` `strings` first (which feels fragile), or build `strings` first, and then build `args` from it. – Martin Bonner supports Monica Jan 12 '16 at 14:07
And another possibility, if the only concern is using value-based containers to avoid deletes on cleanup, is to use a std::set, which is node-based and should not be invalidated by insertions. – Don Wakefield Jan 12 '16 at 14:09
We don't know anything about the strings being used. They might not all be unique, in which case `std::set` is not a good fit. Also the order might matter. – Kevin Jan 12 '16 at 14:11
@Kevin: std::multiset for duplicates, if indeed actual object (char *) addresses need to be unique. Order? These are all being stuffed into structs, so no. – Don Wakefield Jan 12 '16 at 15:08

Better way of building dynamic struct with char*?

4 Answers4