AfxMessageBox - Access violation

Question

Here is what is going on. When I try and run an AfxMessageBox from my CDialog extension class, I get an errror (see below). I've googled the internet but come up short. This is the only place the messagebox fails, and I know the rest of the code works (I stepped through it).

Does anyone know how to fix this?

Thanks in advance!

Error message when AFXMESSAGEBOX opens:

Unhandled exception at 0x014b4b70 in IsoPro.exe: 0xC0000005: Access violation reading location 0x34333345.

Code to launch AfxMessageBox, from within CDialog

LPTSTR temp;
mainPassword.GetWindowText((LPTSTR)temp,100);
CString cstr;
cstr.Format("mainPassword = %s",temp);
AfxMessageBox(cstr);

Code to display CDialog:

CEnterpriseManagementDialog* emd = new CEnterpriseManagementDialog();
emd->Create(IDD_ENTERPRISE_MANAGEMENT_DIALOG);
emd->ShowWindow(SW_SHOW);

You should [read this answer](http://stackoverflow.com/questions/24472174/beginner-c-uninitialized-local-variable/24472249#24472249) as to what the issue is with respect to the pointers being used. — PaulMcKenzie, Nov 23 '16 at 20:25
Once it becomes a CString, I thought it was that object and that was that. How am I wrong? Obviously I am, just curious. — Dan G, Nov 23 '16 at 20:34
It is the call to GetWindowText that is at issue, way before that line that uses `CString` becomes involved. Once the erroneous call to GetWindowText is executed, the corruption of memory has already taken place, — PaulMcKenzie, Nov 23 '16 at 21:09
I understand that. I just figured all CStrings were the same object type. I'm obviously wrong. Thanks! — Dan G, Nov 23 '16 at 21:10
Turn on compiler warnings. You should have gotten a "use of uninitialized variable" warning. — Raymond Chen, Nov 24 '16 at 02:27

Mr.C64 · Accepted Answer · 2016-11-28T18:59:27.093

3

The problem is how you use GetWindowText:

LPTSTR temp;
mainPassword.GetWindowText((LPTSTR)temp,100);

You are letting GetWindowText attempt to write to some unallocated memory passing the uninitialized temp pointer. If you really want to use a raw output buffer, you should allocate room for it before passing a pointer to GetWindowText, e.g.:

TCHAR temp[100];
mainPassword.GetWindowText(temp, _countof(temp));
// NOTE: No need to LPTSTR-cast

But, since you are using C++, you may want to just use a string class like CString, instead of raw buffers, e.g.:

CString password;
mainPassword.GetWindowText(password);

CString msg;
msg.Format(_T("mainPassword = %s"), password.GetString());
// or you can just concatenate CStrings using operator+ ... 
AfxMessageBox(msg);

edited Nov 28 '16 at 18:59

answered Nov 23 '16 at 19:56

Mr.C64

41,637
14
86
162

Brilliant. Thanks! This really should be part of the spec. – Dan G Nov 23 '16 at 20:09
@DanG It is part of the spec. You misread what it means when a function requires a pointer as an argument. When you see such a declaration, it doesn't mean you declare a pointer and pass it to the function. The function wants the *address* of an existing object. – PaulMcKenzie Nov 23 '16 at 20:16
*"write to some unitialized memory"* - It's worse, really. It's writing to random memory through an uninitialized pointer. Writing to uninitialized memory is not an issue in itself. – IInspectable Nov 23 '16 at 22:56
@DanG: Even if it wasn't in the specification ([it is](https://msdn.microsoft.com/library/49a832ee-bc34-4126-88b3-bc1d9974f6c4.aspx#cwnd__getwindowtext)), typing a value for the *nMaxCount* argument should have given it away. Why did you think you need to pass a value, whose only job is to arbitrarily limit the output? – IInspectable Nov 23 '16 at 23:32
*"write to some unallocated memory"* - Again, the fact that no memory was allocated is not at all interesting. The undefined behavior is caused by dereferencing an uninitialized pointer, i.e. the code is writing through an uninitialized pointer. That's undefined behavior even if the memory pointed to were allocated by coincidence. – IInspectable Nov 24 '16 at 19:20
@IInspectable The fact that no memory is allocated is very interesting indeed. In fact, the pointer should point to some allocated memory buffer. That's as simple as that. – Mr.C64 Nov 24 '16 at 22:51
*"The fact that no memory is allocated is very interesting indeed."* - Not as far as C++ is concerned. Dereferencing an **uninitialized** pointer exhibits undefined behavior already. It's completely irrelevant, where it points to. In this case it happened to have the value `0x34333345`, and that memory apparently wasn't readable by the current thread. The undefined behavior manifested itself by raising an access violation. But none of that is interesting as far as the programming language goes. – IInspectable Nov 24 '16 at 23:03

score 1 · Answer 2 · answered Nov 23 '16 at 19:55

1

It looks like the variable temp is an uninitialized pointer (the definition of LPTSTR is a char *).

Try defining temp as an array instead:

TCHAR temp[64];

answered Nov 23 '16 at 19:55

Loring

323
2
12

*"the definition of LPTSTR is a char *"* - It isn't. [LPTSTR](https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751.aspx#LPTSTR) depends on the `UNICODE` preprocessor symbol, and it either expands to a `char*` or a `wchar_t*`. Unless you are maintaining legacy software, the `UNICODE` preprocessor symbol should always be defined. – IInspectable Nov 23 '16 at 22:59

AfxMessageBox - Access violation

2 Answers2