Question

I'm trying to remove all the delete and delete[] of an old application and use smart pointers instead. In the following code snippet, I want to remove the last for cicle.

std::unique_ptr<MapiFileDesc> fileDesc(new MapiFileDesc[numFiles]);

for (int i = 0; i < numFiles; ++i)
{
    // Works but I've to delete[] at the end
    fileDesc[i].lpszPathName = new CHAR[MAX_PATH];

    // Does not work. For each iteration the previous array will be deleted
    // It also happens with shared_array
    boost::scoped_array<CHAR> pathName(new CHAR[MAX_PATH]);
    fileDesc[i].lpszPathName = pathName.get();
}

// I want to remove the following cicle
for (int i = 0; i < numFiles; ++i)
{
    delete [] fileDesc[i].lpszPathName;
    fileDesc[i].lpszPathName = nullptr;
}

What do you think it is the best approach for this situation: use a wrapper object that will keep track of all the arrays created and delete them in the destructor OR use a vector of boost::shared_array and assign them to each of the elements?

std::vector<boost::shared_array<CHAR> > objs;

for (int i = 0; i < 10; ++i)
{
    objs.push_back(boost::shared_array<CHAR>(new CHAR[MAX_PATH]));
}

I need to use boost::shared_ptr since I'm using VC++ 2008

Thanks in advance. J. Lacerda

Was it helpful?

Solution

std::vector<std::string > objs(numFiles, std::string(MAX_PATH, 0));
std::vector<MapiFileDesc> fileDesc(numFiles);
for (int i = 0; i < numFiles; ++i)
    fileDesc[i].lpszPathName=objs[i].data();
// after the C API calls, if you do need to use the strings as C++ strings,
// resync the C++ string length with the C string data
// (not necessary if you just use them via c_str())
for (int i = 0; i < numFiles; ++i)
    objs[i].resize(strlen(objs[i].c_str());

By the way, if you don't need to pass the whole array to C APIs but just single structs, you can make a single vector of a struct that stores both the MapiFileDesc structure and the std::string, binding strongly the lifetime of both objects and allowing the constructor to take care of linking lpszPathName with the string data() member; still, probably I wouldn't bother if this structure is used just in a single function.

OTHER TIPS

std::unique_ptr<MapiFileDesc[]> fileDesc(new MapiFileDesc[numFiles]);
typedef std::unique_ptr<CHAR[]> CharBuffer;
std::vector<CharBuffer> pathNameBuffers;

for (int i = 0; i < numFiles; ++i)
{
    pathNameBuffers.push_back(CharBuffer(new CHAR[MAX_PATH]));
    fileDesc[i].lpszPathName = pathNameBuffers.back().get();
}

This doesn't null out the pointers at the end though.

While trying to reduce the amount of pointers / getting rid of the ugly memory management, reducing the amount of delete and delete[] calls is not the only thing that you can do.

The standard library offers many neat classes that will allow you to work with objects that have automatic storage duration. Use STL containers such as std::vector instead of C-style arrays and for arrays of characters that semantically represent a string, use std::string or std::wstring accordingly.

I like the approach of boost shared array. I think the issue u r facing is get () method in boost shared_array is not incrementing reference count for the object. Here is one work around in your example that will increment the reference count.

for (int i = 0; i < numFiles; ++i)
{
    // Works but I've to delete[] at the end
    fileDesc[i].lpszPathName = new CHAR[MAX_PATH];

    // Does not work. For each iteration the previous array will be deleted
    // It also happens with shared_array
    boost::shared_array<CHAR> pathName(new CHAR[MAX_PATH]);
    fileDesc[i].lpszPathName = pathName.get();

    **// Here is a workaround to increase reference count
    boost::shared_array<CHAR> pathNameTemp (pathName);**
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top