0

I had these two functions in my project:

char* V8StringToChar(v8::Handle<v8::String> str);
char* V8StringToChar(v8::Local<v8::Value> val);

I converted them to:

template <class T>
class ArrayDeleter {
public:
    void operator () (T* d) const
    { delete [] d; }
};
std::shared_ptr<char> V8StringToChar(v8::Handle<v8::String> str);
std::shared_ptr<char> V8StringToChar(v8::Local<v8::Value> val);

with body as

std::shared_ptr<char> V8StringToChar(Handle<String> str) {
  int len = str->Utf8Length();
  char* buf = new char[len + 1];
  str->WriteUtf8(buf, len + 1);
  return std::shared_ptr<char>(buf, ArrayDeleter<char>());
}
std::shared_ptr<char> V8StringToChar(Local<Value> val) {
  return V8StringToChar(val->ToString());
}

And every usage of them to (&*V8StringToChar(whatever)).

And it build perfectly.

And it is causing run time errors.

Is there any cases where this could fail And please provide some good solution ?

3
  • 3
    Use vector<char> for an array of bytes.
    – Neil Kirk
    Commented Sep 5, 2013 at 11:10
  • 2
    The standard already provides an array deleter, use std::default_delete<char[]> (note the [] characters to specify an array, so it uses delete[]) Commented Sep 5, 2013 at 11:15
  • 1
    Probably because &* gives you a raw pointer, which can be invalidated when the array is deleted - in your example, that happens immediately, before you can do anything with the pointer. Only do that when your really need a raw pointer, and be very careful to ensure that nothing keeps hold of the pointer. Commented Sep 5, 2013 at 11:16

1 Answer 1

5

Instead of

(&*V8StringToChar(whatever))

you could have written:

V8StringToChar(whatever).get()

But both are probably wrong and guaranteed to fail in some circumstances.

Doing that creates a new buffer, returns it as a shared_ptr, gets the address of the buffer, then the shared_ptr goes out of scope and the buffer is deleted, leaving you with a dangling pointer. Boom, any attempt to access the memory at that address is undefined behaviour. Go to jail, go directly to jail, do not pass go, do not collect £200.

I would make your functions return a std::unique_ptr<char[]> instead, because that has built-in support for arrays.

std::unique_ptr<char[]> V8StringToChar(Handle<String> str) {
  int len = str->Utf8Length();
  std::unique_ptr<char[]> buf(new char[len + 1]);
  str->WriteUtf8(buf.get(), len + 1);
  return buf;
}
std::unique_ptr<char[]> V8StringToChar(Local<Value> val) {
  return V8StringToChar(val->ToString());
}

To fix the run-time failures you must keep the smart pointer around as long as the buffer is needed e.g.

std::unique_ptr<char[]> smartptr = V8StringToChar(whatever);
char* ptr = smartptr.get());
doSomethingWithPtr(ptr);
// now it's OK if `smartptr` goes out of scope
8
  • if i do V8StringToChar(whatever).get() everywhere that should be fine too ? But its not working yet. Commented Sep 5, 2013 at 11:37
  • @Jonathan Wakely Would V8StringToChar(whatever).get() is right ? Commented Sep 5, 2013 at 11:48
  • @ASHISHNEGI, No, that is wrong too! I said "But both are probably wrong and guaranteed to fail in some circumstances." Please try to understand the problem I described (and Mike Seymour described in his comment above): the smart pointer goes out of scope and so deletes the buffer at the end of the statement. Commented Sep 5, 2013 at 11:48
  • @Jonathan Wakely I mean after using your solution unique_ptr<char[]> if i do use .get() like some_other_func(V8StringToCHar(..).get()) <- is it wrong ? Commented Sep 5, 2013 at 12:02
  • YES IT'S STILL WRONG. The solution is shown at the end of my answer where I say "To fix the run-time failures", did you read that part?! Just switching to unique_ptr and using .get() doesn't address the problem of the smart pointer going out of scope. Please try to understand the problem and engage brain. Commented Sep 5, 2013 at 12:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.