Memory management for fetching a list of drive letters on Windows

Question

I'm using the WinAPI GetLogicalDriveStrings() function that requires a LPWSTR and am wondering if there is a safer way to do this to ensure there is no memory leak.

Currently I construct an initial pointer to a buffer buf using:

auto buf = GetLogicalDriveStrings(0, nullptr);

Then I create the LPWSTR to be used in lieu of the null pointer in my actual call using:

auto driveStrings = static_cast<LPWSTR>(malloc((buf + 1) * sizeof(WCHAR)));

Next I create a pointer to driveStrings to free it later. After checking if driveStrings is a null pointer or if the buffer (buf) is NULL (in case memory couldn't be allocated), I call GetLogicalDriveStrings() using driveStrings.

After I get the result I manually free() the LPWSTR using the pointer I made after allocating it.

How can I use a smart pointer for LPWSTR instead so I don't have to use malloc() and free(), but so that it will still work with the GetLogicalDriveStrings() function?

Mininum working example:

auto buf = GetLogicalDriveStrings(0, nullptr);

auto driveStrings = static_cast<LPWSTR>(malloc((buf + 1) * sizeof(WCHAR)));
auto pDriveStrings = driveStrings;

if (driveStrings == nullptr || buf == NULL)
{
    std::stringstream msg;
    msg << "Can't allocate memory for drive list: ";
    msg << GetLastError();
    throw std::runtime_error(msg.str());
}

// get drive strings
if (GetLogicalDriveStrings(buf, driveStrings) == NULL)
{
    std::stringstream msg;
    msg << "GetLogicalDriveStrings error: ";
    msg << GetLastError();
    throw std::runtime_error(msg.str());
}

// iterate over results
while (*driveStrings)
{
    // GetDriveType() requires a LPCWSTR
    if (GetDriveType(driveStrings) == DRIVE_FIXED || GetDriveType(driveStrings) == DRIVE_REMOVABLE)
    {
        std::wcout << driveStrings << std::endl;
    }
    driveStrings += lstrlen(driveStrings) + 1;
}

free(pDriveStrings);

If I use a std::wstring, I can't figure out how to iterate over each string in the driveStrings buffer. If I use a std::vector<WCHAR>, I can't figure out how to cast each element to an LPCWSTR for GetDriveType().

This works fine as is, but is there a better/safer way to do this? I'm open to any and all improvements.

_{Cross-posted on StackOverflow, but it has close votes because it is a better fit here.}

Answer 1

Your code is leaking memory. If you throw an exception, free no longer gets called. Using a std::vector makes sure the memory is freed.

Iterating through the array returned is quite straight-forward, considering that each drive letter has a null-terminator and than the whole vector has an extra null-terminator (requiredSize = writtenChars + 1)

auto requiredSize = GetLogicalDriveStringsW(0, nullptr);

std::vector<WCHAR> buffer(requiredSize + 1);
auto writtenChars = GetLogicalDriveStringsW(requiredSize, buffer.data());

if (writtenChars > requiredSize)
{
    //MSDN: If the buffer is not large enough, the return value is greater than nBufferLength.
}

if (writtenChars == 0)
{
    //handle error
}

const WCHAR* currentString = buffer.data();

while (*currentString != 0)
{
    std::wcout << currentString << L'\n';
    //pass currentString to other methods
    currentString += wcslen(currentString) + 1;
}