CODEWITHSUNDEEP
c++optimizationmemory-management
Matt Chambers
Matt Chambers

Reputation: 2346

Converting a char* returned by C API to C++ string

I found this code in a C++ header-only wrapper around a C API I'm working with:

static string GetString(const char* chString)
{
    string strValue;
    if (NULL != chString)
    {
        strValue.swap(string (chString));
        releaseMemory((void*&)chString);
        chString = NULL;
    }
    return strValue;
}

I suppose the author is trying to give the string strValue ownership of chString and then free the empty buffer. I suspect this is very wrong (including it being const char*), but it actually seems to work with MSVC 12. At least I haven't seen it crash spectacularly yet.

Assuming that the C API and the C++ library are using the same heap (so that the string can reallocate the buffer if necessary and eventually release it), is there a way to properly achieve this? How about this?

template <typename T> struct Deleter { void operator()(T o) { releaseMemory((void*&)o); } };

static std::string GetString(char* chString)
{
    if (NULL == chString)
        return std::string();
    return std::string(std::unique_ptr<char[], Deleter<char[]>>(chString).get());
}

Again, assuming the C API is using the same heap as std::string.

If that's also very wrong, then is there an immutable, owning C-style string wrapper? Something like string_view but immutable (so const char* input would be ok) and owning (so it deletes the C string, possibly with a custom deleter, in its dtor)?

Upvotes: 0

Views: 126

Answers (2)

Matt Chambers
Matt Chambers

Reputation: 2346

Does this seem like a good solution for my last question (and ultimate goal of being able to use a char* like a string without copying it)?

template <typename DeleterT = std::default_delete<const char*>>
class c_str_view
{
    public:
    unique_ptr<const char*, DeleterT> strPtr_;
    size_t len_;

    c_str_view() {}
    c_str_view(const char* charPtr) : strPtr_(charPtr), len_(strlen(charPtr)) {}
    c_str_view(const char* charPtr, size_t len) : strPtr_(charPtr), len_(len) {}

    operator std::string_view () const
    {
        return string_view(strPtr_.get(), len_);
    }
};

If so, is there a good reason this isn't in the upcoming standard since string_view is coming? It only makes sense with string_view of course, since any conversion to std::string would cause a copy and make the whole exercise pointless.

Here's a test: http://coliru.stacked-crooked.com/a/9046eb22b10a1d87

Upvotes: 0

Remy Lebeau
Remy Lebeau

Reputation: 596287

I suppose the author is trying to give the string strValue ownership of chString and then free the empty buffer.

No. It makes an (inefficient and error-prone) copy of the character data pointed to by chString, then releases the memory pointed to by chString (which will be skipped if the copy throws an exception), and then returns the copy.

Assuming that the C API and the C++ library are using the same heap

That is not a correct assumption, or even a necessary one. The copy can use whatever heap it wants.

is there a way to properly achieve this? How about this?

You are on the right track to use a std::unique_ptr with a custom deleter, but there is no reason to use the T[] array specialization of std::unique_ptr.

The code can be simplified to something more like this:

void Deleter(char* o) { releaseMemory((void*&)o); }

static std::string GetString(char* chString)
{
    std::string strValue;
    if (chString) {
        std::unique_ptr<char, decltype(&Deleter)>(chString, &Deleter);
        strValue = chString;
    }
    return strValue;
}

Or, just get rid of the check for chString being null, it is not actually needed. std::string can be constructed from a null char*, and std::unique_ptr will not call its deleter with a null pointer:

void Deleter(char* o) { releaseMemory((void*&)o); }

static std::string GetString(char* chString)
{
    std::unique_ptr<char, decltype(&Deleter)>(chString, &Deleter);
    return std::string(chString);
}

Upvotes: 1

Related Questions