Return array of wchar_t causing memory leak

Question

I am trying to generate a JSON string from my input arrays . I have used "new" inorder to allocate the memory for this JSON. But I am not sure where to deallocate this memory or if there is a better way to write this function .

    wchar_t*  SetExpectedTabsData(_In_ PCWSTR tabUrls[], _In_ PCWSTR tabTitles[], _In_ UINT tabsCount)
{
    wchar_t* tabsDataJSON = new wchar_t[JSONTABDATASIZE], jsonPerTab[256];
    StringCchPrintf(tabsDataJSON, JSONTABDATASIZE, L"\"tabs\":[");
    bool isActiveTab = true;
    for (int i = 1; i <= tabsCount; ++i)
    {
        StringCchPrintf(jsonPerTab, ARRAYSIZE(jsonPerTab), L"{\"id\":%i,\"index\":%i,\"windowId\":1,\"active\":%s,\"status\":\"complete\",\"title\":\"%s\",\"url\":\"%s\"}", i, (i - 1), isActiveTab ? L"true" : L"false", tabTitles[i - 1], tabUrls[i - 1]);
        StringCchCat(tabsDataJSON, JSONTABDATASIZE, jsonPerTab);
        isActiveTab = false;
        if (i != tabsCount)
        {
            StringCchCat(tabsDataJSON, JSONTABDATASIZE, L",");
        }
    }
    StringCchCat(tabsDataJSON, JSONTABDATASIZE, L"],");
    return tabsDataJSON;
}

Please suggest.

I am not sure how to do that. I need to return the same thing. Is there something like finally where i can do the clean up , after the value is returned — jetty, Mar 14 '16 at 14:51
Man, I don't know what language this is, but it is not C++... jokes aside, just use std::vector. — Slava, Mar 14 '16 at 14:52
@jetty That is what I was getting at. Once you are done with the returned pointer then you delete it. I would follow Joachim Pileborg's advice though and just use a `std::wstring`. — NathanOliver, Mar 14 '16 at 14:53

score 1 · Answer 1 · edited Mar 14 '16 at 15:10

    wchar_t*  SetExpectedTabsData(_In_ PCWSTR tabUrls[], _In_ PCWSTR tabTitles[], _In_ UINT tabsCount)
{
    wchar_t* tabsDataJSON = new wchar_t[JSONTABDATASIZE], jsonPerTab[256];
    StringCchPrintf(tabsDataJSON, JSONTABDATASIZE, L"\"tabs\":[");
    bool isActiveTab = true;
    for (int i = 1; i <= tabsCount; ++i)
    {
        StringCchPrintf(jsonPerTab, ARRAYSIZE(jsonPerTab), L"{\"id\":%i,\"index\":%i,\"windowId\":1,\"active\":%s,\"status\":\"complete\",\"title\":\"%s\",\"url\":\"%s\"}", i, (i - 1), isActiveTab ? L"true" : L"false", tabTitles[i - 1], tabUrls[i - 1]);
        StringCchCat(tabsDataJSON, JSONTABDATASIZE, jsonPerTab);
        isActiveTab = false;
        if (i != tabsCount)
        {
            StringCchCat(tabsDataJSON, JSONTABDATASIZE, L",");
        }
    }
    StringCchCat(tabsDataJSON, JSONTABDATASIZE, L"],");
    return tabsDataJSON;
}

In main for example:

int main()
{
    wchar_t* tabsDataJSON = SetExpectedTabsData(tabUrls, tabTitles, tabsCount);
    //do something with tabsDataJSON
    //..

    //end of scope of tabsDataJSON
    //DONT FORGET TO DELETE IT ON THE RIGHT POINT IF YOU DONT NEED IT ANYMORE
    delete[] tabsDataJSON;
    return 1;
}

I am calling the SetExpectedTabsData(...) . How do i access a local variable tabsDataJSON of the function? — jetty, Mar 14 '16 at 15:10
wchar_t* tabsDataJSON = SetExpectedTabsData(tabUrls, tabTitles, tabsCount); is the Point :) @Seb: Thanks... I think I was a bit too lazy... — H. Senkaya, Mar 14 '16 at 15:18

score 1 · Answer 2 · edited May 23 '17 at 12:15

You are rightly concerned. When you use new then you have taken on the responsibility to call delete at the right time.

The best solution would probably be to use a std::unique_ptr. The pointer can be moved out of the function that created it and it will be deleted and freed automatically according to RAII principles when it goes out of scope.

Create the std::unique_ptr in the function:

std::unique_ptr<wchar_t[]>  SetExpectedTabsData(_In_ PCWSTR tabUrls[],_In_ PCWSTR tabTitles[],_In_ UINT tabsCount)
{
    //wchar_t* tabsDataJSON = new wchar_t[JSONTABDATASIZE],jsonPerTab[256];
    std::unique_ptr<wchar_t[]> tabsDataJSON{new wchar_t[JSONTABDATASIZE]};
    wchar_t jsonPerTab[256];
    StringCchPrintf(tabsDataJSON.get(),JSONTABDATASIZE,L"\"tabs\":[");
    bool isActiveTab = true;
    for (int i = 1; i <= tabsCount; ++i) {
        StringCchPrintf(jsonPerTab,ARRAYSIZE(jsonPerTab),L"{\"id\":%i,\"index\":%i,\"windowId\":1,\"active\":%s,\"status\":\"complete\",\"title\":\"%s\",\"url\":\"%s\"}",i,(i - 1),isActiveTab ? L"true" : L"false",tabTitles[i - 1],tabUrls[i - 1]);
        StringCchCat(tabsDataJSON.get(),JSONTABDATASIZE,jsonPerTab);
        isActiveTab = false;
        if (i != tabsCount) {
            StringCchCat(tabsDataJSON.get(),JSONTABDATASIZE,L",");
        }
    }
    StringCchCat(tabsDataJSON.get(),JSONTABDATASIZE,L"],");
    return tabsDataJSON;
}

Then it gets moved out to your variable in the main program:

int main()
{
    std::unique_ptr<wchar_t[]> tabsDataJSON = SetExpectedTabsData(tabUrls, tabTitles, tabsCount);
    //do something with tabsDataJSON
    //..

    //no need to remember delete[]
    return 1;
}

Also keep in mind that you may want to pass a "plain" version of this pointer to another function at some point. In that case you may call some_function_that_uses_plain_array_pointer(tabsDataJSON.get());. Or, if you want to "hand back control" of the pointer to a function that only knows about plain pointers and which will be expected to call delete for you, then you can use some_function_that_releases_plain_array_pointer(tabsDataJSON.release());.

score 0 · Answer 3 · edited Mar 21 '16 at 10:36

0

// call function
wchar_t* mystr = SetExpectedTabsData(...);
// use mystr
// delete it
delete [] mystr;

edited Mar 21 '16 at 10:36

Ilya

4,583
4
26
51

answered Mar 14 '16 at 14:55

Sven Nilsson

1,861
10
11

Will this free memory allocated for tabsDataJSON ? I have been a java coder for far too long. – jetty Mar 14 '16 at 15:04

Return array of wchar_t causing memory leak

3 Answers3