CODEWITHSUNDEEP
c++winapi
celphato
celphato

Reputation: 112

GetWindowTextA returns gibberish with unrelated code changes

I've tried this code which was supposed to get all window titles and positions and store them in vectors (here window titles are printed) but the outputs seemed completely random:

#include <windows.h>
#include <stdio.h>
#include <iostream>
#include <vector>

std::vector<LPSTR> buffs;
std::vector<int> rectposs;

BOOL CALLBACK EnumWindowsProc(HWND hWnd, long lParam)
{
    LPSTR buff;

    if (IsWindowVisible(hWnd))
    {
        GetWindowTextA(hWnd, buff, 254);
        buffs.push_back(buff);
        RECT rect;
        if (GetWindowRect(hWnd, &rect))
        {
            rectposs.push_back(rect.left);
            rectposs.push_back(rect.right);
            rectposs.push_back(rect.top);
            rectposs.push_back(rect.bottom);
        }
    }

    return TRUE;
}

int main()
{
    EnumWindows(EnumWindowsProc, 0);
    for (LPSTR buff : buffs)
    {
        std::cout << buff << std::endl;
    }
    return 0;
}

I expected the output to contain lines like Settings and Alarms and Clock since I had them open but instead everything was along the lines of ���$, and what confused me was if I removed lines 20-23 (push back the positions of windows) the problem was apparently fixed, which shouldn't be the case since it has nothing to do with GetWindowTextA or how I saved the window titles. As such I wasn't able to produce a minimum reproducible example since seemingly unrelated code seemed to change the output completely.

Are my suspicions that this has something to do with lines 20-23 overwriting the window titles correct? If so or otherwise how can I make sure it doesn't happen and still have the data I want?

Upvotes: 0

Views: 85

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 595349

Your buff variable is an uninitialized pointer, it doesn't point anywhere meaningful. So your call to GetWindowTextA() is writing to random memory. To fix that, you need to allocate actual memory for it to write to.

After fixing that, you have another problem - you are pushing the same pointer into the buffs vector on each iteration. So, once the enumeration is finished, all entries will be pointing to the same memory, which will hold the result of the last call to GetWindowTextA(). To fix that, you need to make a copy of the buff data on each push. The easiest way to solve that is to change your buffs vector to hold std::string values instead of LPSTR pointers.

Lastly, I suggest changing the rectposs vector to hold actual RECT objects instead of the individual int values (though, you should consider defining a new struct/class to hold all of the window info you want, and then use a single vector to hold objects of that type).

Try this:

#include <iostream>
#include <vector>
#include <string>
#include <windows.h>

std::vector<std::string> buffs;
std::vector<RECT> rectposs;

BOOL CALLBACK EnumWindowsProc(HWND hWnd, long lParam)
{
    if (IsWindowVisible(hWnd))
    {
        CHAR buff[255]{};
        GetWindowTextA(hWnd, buff, 254);
        buffs.push_back(buff);

        RECT rect;
        GetWindowRect(hWnd, &rect);
        rectposs.push_back(rect);
    }

    return TRUE;
}

int main()
{
    EnumWindows(EnumWindowsProc, 0);
    for (string& buff : buffs)
    {
        std::cout << buff << std::endl;
    }
    return 0;
}

Upvotes: 1

Related Questions