CODEWITHSUNDEEP
c++winapi
iNCEPTiON_
iNCEPTiON_

Reputation: 621

Convert char to std::string not working

I'm trying to list the logical drives of my Pc and add the result to a vector but I'm getting this weird result = ;;;.

What am i missing here? 

std::vector<std::string> directory::getLogicalDrives() {
    DWORD mydrives = 100;
    char lpBuffer[100];
    DWORD drives = GetLogicalDriveStrings(mydrives, lpBuffer);

    std::vector<std::string> driveList;
    for (int i = 0; i < 100; i++) {
        std::string drive(3, '%c' + lpBuffer[0]); // Missing something?
        driveList.push_back(drive);
    }
    return driveList;
}

Upvotes: 0

Views: 360

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 598134

DWORD mydrives = 100;
char lpBuffer[100];

100 is not enough characters. Technically, a computer can have up to 26 drive letters (realistically, no one has that many drive letters at one time, but you should still prepare for it). A 100-character buffer has enough space to receive 24 drive letter strings at most (4 * 24 + 1 = 97, 4 * 25 + 1 = 101). You need space for at least 105 characters in your buffer to receive 26 drive letter strings (4 * 26 + 1 = 105).

std::string drive(3, '%c' + lpBuffer[0]); // Missing something?

This line does not do what you think it does. You are thinking of the C-style sprint() function instead. You can't use formatting strings with std::string like this. This line should not even compile, or should compile with warnings.

Besides, you are not even looping through the output strings correctly anyway. GetLogicalDriveStrings() returns a double-terminated list of strings in <drive>:\<nul> format, where <drive> is <nul> at the end of the list. For example, if drives A, B, and C are returned, the contents of the buffer will look like this:

lpBuffer[ 0] = 'A';
lpBuffer[ 1] = ':';
lpBuffer[ 2] = '\\';
lpBuffer[ 3] = '\0';
lpBuffer[ 4] = 'B';
lpBuffer[ 5] = ':';
lpBuffer[ 6] = '\\';
lpBuffer[ 7] = '\0';
lpBuffer[ 8] = 'C';
lpBuffer[ 9] = ':';
lpBuffer[10] = '\\';
lpBuffer[11] = '\0';
lpBuffer[12] = '\0';

The correct approach is to loop through the buffer in groups of 4 characters until you reach that final null terminator (and don't forget error checking!), eg:

DWORD drives = GetLogicalDriveStrings(mydrives, lpBuffer);
if ((drives > 0) && (drives <= mydrives)) {
    char *pdrive = lpBuffer;
    while (*pdrive) {
        std::string drive(pdrive);
        driveList.push_back(drive);
        pdrive += 4;
    }
}

Alternatively:

DWORD drives = GetLogicalDriveStrings(mydrives, lpBuffer);
if ((drives > 0) && (drives <= mydrives)) {
    for (int i = 0; i < drives; i += 4) {
        std::string drive(&lpBuffer[i]);
        driveList.push_back(drive);
    }
}

That being said, consider using GetLogicalDrives() instead, then you don't have to worry about allocating memory for, or looping through, any drive letter strings at all:

DWORD drives = GetLogicalDrives();
if (drives) {
    char drive[] = "?:\\";
    for (int i = 0; i < 26; i++) {
        if (drives & (1 << i)) {
            drive[0] = 'A' + i;
            driveList.push_back(drive);
        }
    }
}

Upvotes: 1

Slava
Slava

Reputation: 44278

As stated in the documentation GetLogicalDriveStrings() gives you a list of NULL-terminated strings, and the list is terminated by a NULL character. So just iterate that list, eg:

std::vector<std::string> directory::getLogicalDrives() {
    std::vector<std::string> driveList;
    char szBuffer[105];
    DWORD size = GetLogicalDriveStrings(104, szBuffer);
    if ((size > 0) && (size <= 104)) {
        const char *pstr = szBuffer;
        while( *pstr ) {
            std::string drive( pstr );
            driveList.push_back(drive);
            pstr += (drive.length() + 1);
        }
    }
    return driveList;
}

Upvotes: 7

Related Questions