1

I need to use an API as part of a project that includes a class called ParamSet with a method that is defined as the following:

void AddString(const std::string &, std::unique_ptr<std::string[]> v, int nValues);

The purpose of the method is to add an array of strings to the object to describe some parameter. For example a ParamSet object could require a "filename" parameter that points to an array of a nValues strings.

However, when I try and pass the method a unique_ptr to an array containing only 1 string, the code seg faults upon calling the destructor for the ParamSet object unless I define the unique_ptr in a specific way.

The following code causes a seg-fault upon calling Clear() or on return.

ParamSet badparam;
badparam.AddString("filename", unique_ptr<string[]> (new string("test")), 1);
badparam.Clear(); // <------ CAUSES SEG FAULT

However the following does not cause a seg fault.

ParamSet testparam;
std::unique_ptr<std::string[]> strings(new std::string[0]); // DOH, should be string[1]
strings[0] = std::string("test");
testparam.AddString("filename", std::move(strings), 1);
testparam.Clear(); // <------ NO SEG FAULT

I don't understand why creating the unique_ptr in the line calling AddString leads to a seg fault, but creating it outside of the call to does not.

Not a real meerkat
  • 5,604
  • 1
  • 24
  • 55
viliam
  • 13
  • 3
  • 1
    Neither of those codes allocates a `std::string` array with one element. – tkausl Sep 26 '18 at 22:54
  • 2
    You must use `new[]` if allocating for array form of unique_ptr – M.M Sep 26 '18 at 22:54
  • 3
    If the API just used `std::vector`, this would be way easier. – o11c Sep 26 '18 at 23:06
  • 1
    It'd be nice if you could do something like `make_unique("foo", "bar")`, but the standard has outlawed that – M.M Sep 26 '18 at 23:08
  • @o11c this seems like a good use case for [span](https://stackoverflow.com/questions/45723819/what-is-a-span-and-when-should-i-use-one), then the caller is not forced to use a particular container (be it `unique_ptr[]`, or `vector` etc.) – M.M Sep 26 '18 at 23:14
  • @M.M perhaps, though that would require `AddString()` to make a copy of whatever data is passed in, instead of take owrnship of it. – Remy Lebeau Sep 27 '18 at 15:05

2 Answers2

1

The problem is that you use non-array new to allocate memory managed by unique_ptr<T[]> which will then go on to use array delete[], causing undefined behaviour.

The syntax could be:

badparam.AddString("filename", std::unique_ptr<std::string[]>(new std::string[1]{"test"}), 1);
M.M
  • 138,810
  • 21
  • 208
  • 365
0

The array specialization of std::unique_ptr requires the array to be allocated with new[], as it uses delete[] by default to free the array.

In your first example, you are allocating a single std::string object with new, not a 1-element array with new[].

In your second example, you are allocating a 0-element std::string array, but you need a 1-element array instead.

Try this instead:

std::unique_ptr<std::string[]> strings(new std::string[1]); // <-- NOT 0!
// Or, if you are using C++14:
// auto strings = std::make_unique<string[]>(1);
strings[0] = "test";
testparam.AddString("filename", std::move(strings), 1);

Alternatively:

testparam.AddString("filename", std::unique_ptr<std::string[]>(new std::string[1]{"test"}), 1);
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Thanks! This syntax is exactly what I was searching for. I need C++11 for now so I'd need to write my own make_unique, but that's not so bad. The string[0] is my facepalm moment for the day... – viliam Sep 27 '18 at 14:54