Why is the code that grabs a window title causing an app crash?

Question

I'm very new to C++ but have made a console application that waits until it detects a window with a certain title and then executes some code. It's working really well as long as it detects a window. However, I've noticed that if the application is run and the window doesn't appear then, after 4 minutes and 20 seconds (approx.) it crashes.

The crash information simply says APPCRASH attributed to ntdll.dll

I've put breakpoints in the code and narrowed it down to this piece of code that is causing the crash.

// GRAB WINDOW TITLE AND PUT IT IN "lpszTitle"
int nLen = GetWindowTextLength(hWnd);
LPSTR lpszTitle = new CHAR[nLen + 1];
GetWindowText(hWnd, lpszTitle, nLen);

This code is followed by an "if" statement that executes the main code when the window title matches a certain string.

Until then, this code just runs on a while(1) loop waiting.

Here is a more complete listing of the code.

#pragma once
#undef UNICODE
#include <windows.h>
#include <iostream>
#include <string>
#include <sstream>
#include <tchar.h>
#include <cassert>
#include <cstdio>
#include <stdlib.h>
#include <mmsystem.h>
#include <time.h>


// Example of the __stdcall keyword  
#define WINAPI __stdcall

using namespace std;

HMODULE hLib;
BOOL CALLBACK SearchProc(HWND hWnd, LPARAM lParam)
{

// THIS FUNCTION SEARCHES ALL OPEN WINDOWS
// UNTIL IT FINDS ONE WITH "DX" IN IT'S TITLE
// IT THEN GRABS THE MEMORY ADDRESS REFERENCED
// BELOW WHICH CONTAINS THE "TOTALOUT" VARIABLE
// FOR THE FRUIT MACHINE




// INITIALISE VARIABLES FOR USE IN THIS FUNCTION
DWORD address = 0x96e0a4;
long currentMachinePaidOutTotal = 0;
long unsortedTotalcurrentMachinePaidOutTotal = 0;
long calculatedWinnings = 0;    
long oldAmountMachinePaidOut = 0;
bool initialiseOldAmountMachinePaidOut = false;
bool slowMachine = false;
bool payoutFiverOneShot = false;
string currentWindowName = "";
DWORD pid;

// GRAB WINDOW TITLE AND PUT IT IN "lpszTitle"
int nLen = GetWindowTextLength(hWnd);
LPSTR lpszTitle = new CHAR[nLen + 1];
GetWindowText(hWnd, lpszTitle, nLen);


// LET'S CHECK IF WINDOW HAS "DX" IN TITLE
if (strstr(lpszTitle, "DX") && !strstr(lpszTitle, "=")) {
    cout << "\nFound window with name: " << lpszTitle << "\n";
    currentWindowName = lpszTitle;

    // Found "DX" in the title of the window
    // CONTINUE WITH CODE IN HERE....

    return TRUE;
}
else {
    // NO WINDOW WITH "DX" IN THE TITLE OPEN YET
}

return TRUE;
}



int main(int argc, char *argv[])
{

hLib = LoadLibrary("cash.dll");
assert(hLib != NULL); // pass !!


while (1)
{
    EnumWindows(SearchProc, NULL);
}
return 0;
}

Any ideas why it would cause the crash?

If you need to see more code or get more information then please comment. I've deliberately only included the 'problem' piece of code for brevity and clarity.

Answer 1

You allocate lpszTitle with new without ever deallocating it. Depending on how often the while-loop runs, you could run out of memory quite fast. Try to add a delete[] lpszTitle before the end of the function.

Some other tidbits:

1. You seem to be mixing BOOL with bool. I would suggest going with bool, as it is standard C++.

2. Manual memory allocation is old-school C++. Try out smart pointers.

3. Why not use std::string (or std::wstring as suggested by @IInspectable in comments) instead of char[]?

4. Declare variables when they are used, and not all in the start of the function (you might be coming from fortran?).

5. Why is "using namespace std" considered bad practice?

However, I would suggest you post your final code to https://codereview.stackexchange.com for more and better input.