0

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;
}
Govind Parmar
  • 20,656
  • 7
  • 53
  • 85
iNCEPTiON_
  • 611
  • 2
  • 10
  • 27
  • Did you try stepping through your code, with a debugger? – Algirdas Preidžius Feb 08 '17 at 17:03
  • Of course that's what I always do im getting this ";;;" in every entry. – iNCEPTiON_ Feb 08 '17 at 17:04
  • 6
    '%c' + lpBuffer[0] - what this suppose to do? – Slava Feb 08 '17 at 17:05
  • why do you loop 100 times. What is `i` for. What is the %c for. First thing: Make your code simply cout the drive names – pm100 Feb 08 '17 at 17:06
  • @iNCEPTiON_ Did you inspect the values of your variables, at every step, as you were stepping through your code? At which line, did the code do what you didn't expect? What exactly did it do, that you didn't expect? – Algirdas Preidžius Feb 08 '17 at 17:06
  • 1
    What's the value of `lpBuffer[0]` when `'%c' + lpBuffer[0]` resolves to `;`? – George Feb 08 '17 at 17:06
  • 2
    read the spec of getdrivestrings "A pointer to a buffer that receives a series of null-terminated strings, one for each valid drive in the system, plus with an additional null character. Each string is a device name.". You have no code to deal with the format – pm100 Feb 08 '17 at 17:07
  • If I loop through the char array I'm getting the right values my drives. – iNCEPTiON_ Feb 08 '17 at 17:07
  • 1
    or use this as a starter http://stackoverflow.com/questions/18572944/getlogicaldrivestrings-and-char-where-am-i-doing-wrongly – pm100 Feb 08 '17 at 17:10
  • The code works as expected when using char . I'm a beginner and I've tried to understand converting char to string – iNCEPTiON_ Feb 08 '17 at 17:12
  • @iNCEPTiON_ It looks like you need to read a [good C++ book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) instead of trying to hack something together. And learn how to use a debugger, since from your responses, it was pretty clear, that you didn't even try to use a debugger: [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Algirdas Preidžius Feb 08 '17 at 17:17

2 Answers2

7

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;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Slava
  • 43,454
  • 1
  • 47
  • 90
  • This will only work, in case you set up your project to use MBCS encoding. There is no valid reason not to use Unicode that I am aware of. You should call `GetLogicalDriveStringsW`, replace `char` with `wchar_t`, and `std::string` with `std::wstring`. – IInspectable Feb 08 '17 at 19:45
  • @IInspectable I replied to specific OP question, you should suggest that replacement to OP, not me. – Slava Feb 08 '17 at 19:56
1
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);
        }
    }
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770