Heap Corruption Detected Why?

Mar 4, 2012 at 10:15pm
adn258 (73)
Okay so I'm enumerating a shortcut and this does seems to work so this has a few questions attached.


1
2
3
4
5
6
7
8
9
10
11
 
//needed function
CoInitialize(NULL);

wchar_t* bn = new wchar_t();


ResolveIt(NULL, "C:\\Users\\austin\\Desktop\\Pidgin.lnk",bn,sizeof(bn));

wcout << bn << endl;

delete bn;


Is the code that's causing the error and this function is from


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
 
HRESULT ResolveIt(HWND hwnd, LPCSTR lpszLinkFile, LPWSTR lpszPath, int iPathBufferSize) 
{ 
    HRESULT hres; 
    IShellLink* psl; 
    WCHAR szGotPath[MAX_PATH]; 
    WCHAR szDescription[MAX_PATH]; 
    WIN32_FIND_DATA wfd; 
 
    *lpszPath = 0; // Assume failure 

    // Get a pointer to the IShellLink interface. It is assumed that CoInitialize
    // has already been called. 
    hres = CoCreateInstance(CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, IID_IShellLink, (LPVOID*)&psl); 
    if (SUCCEEDED(hres)) 
    { 
        IPersistFile* ppf; 
 
        // Get a pointer to the IPersistFile interface. 
        hres = psl->QueryInterface(IID_IPersistFile, (void**)&ppf); 
        
        if (SUCCEEDED(hres)) 
        { 
            WCHAR wsz[MAX_PATH]; 
 
            // Ensure that the string is Unicode. 
            MultiByteToWideChar(CP_ACP, 0, lpszLinkFile, -1, wsz, MAX_PATH); 
 
            // Add code here to check return value from MultiByteWideChar 
            // for success.
 
            // Load the shortcut. 
            hres = ppf->Load(wsz, STGM_READ); 
            
            if (SUCCEEDED(hres)) 
            { 
                // Resolve the link. 
                hres = psl->Resolve(hwnd, 0); 

                if (SUCCEEDED(hres)) 
                { 
                    // Get the path to the link target. 
                    hres = psl->GetPath(szGotPath, MAX_PATH, (WIN32_FIND_DATA*)&wfd, SLGP_SHORTPATH); 

                    if (SUCCEEDED(hres)) 
                    { 
                        // Get the description of the target. 
                        hres = psl->GetDescription(szDescription, MAX_PATH); 

                        if (SUCCEEDED(hres)) 
                        {
                            hres = StringCbCopy(lpszPath, iPathBufferSize, szGotPath);
                            if (SUCCEEDED(hres))
                            {
                                // Handle success
                            }
                            else
                            {
                                // Handle the error
                            }
                        }
                    }
                } 
            } 

            // Release the pointer to the IPersistFile interface. 
            ppf->Release(); 
        } 

        // Release the pointer to the IShellLink interface. 
        psl->Release(); 
    } 
    return hres; 
}


You will notice that this is supposed to be used with a window handle but since I am doing this in a console application I set the first to
ResolveIt(NULL

since there's no window handle. This DOES seem to work though since if I allocate memory like so

1
2
3
4
5
6
7
 
wchar_t bn[250];


ResolveIt(NULL, "C:\\Users\\austin\\Desktop\\Pidgin.lnk",bn,250);

wcout << bn << endl;


If gives me the PATH of the .exe file that the shortcut is linked too.

I just wouldn't think that dynamically allocating memory would be a problem for such a small link path would you? What am I missing here? I'm assuming it's allocating more memory then I have but why? The exact error is:

HEAP CORRUPTION DETECTED AFTER normal BLOCK #129 at 0x00287928 CRT detected that this application wrote after end of heap buffer
Mar 4, 2012 at 11:32pm
Disch (13742)
wchar_t* bn = new wchar_t();

You are only allocating one wchar_t. That is enough to hold a string with a length of zero (only the null terminator). If you write any non-empty string to that pointer you will go out of bounds and corrupt the heap.

Also...

sizeof(bn)

bn is a pointer. sizeof(a_pointer) will give you the size of a pointer (ie: 4 or 8 depending on your system), which is useless to you.


You want an array, not a pointer
And you want to give an array size, not the size of a pointer:

1
2
3
4
 
wchar_t bn[256];  // <- an array


ResolveIt(NULL, "C:\\Users\\austin\\Desktop\\Pidgin.lnk", bn, 256 ); // <- the array has 256 elements
Mar 5, 2012 at 12:35am
adn258 (73)
Okay I get that now phew confusing these wchar_t.

So now I want to convert wchar_t to string. and when you use something like 
1
2
3
 
//array for profile path
wchar_t path[250];
ExpandEnvironmentStringsW(L"%USERPROFILE%",path,250);


That does give the USERPROFILE path and if you wcout it out then that will show the user path but if you try outputting this into a normal string via something like

1
2
3
4
5
6
 
for (int i = 0; i < 255; i++)
{
   cout << path[i] << endl;

}


Every part of the array wchat_t's of path HAS a number value though...Why? I would think it would end with that null character?

So to convert it to a standard string I concocted 
1
2
3
4
5
6
7
8
9
10
11
12
 
string all;
for (int i = 0; i < 255; i++)
{
	if (path[i] < 10000 && path[i] != 0)
	{
		
		all += path[i];


	}
}
cout << all << endl;


That does work on my system but that seems absurd to me to have to go through that and I'm sure I'm making booboo's on how hard this stuff is.

I need to convert that to a string or a LPCSTR so the user path can be used for other WIn32 functions

like

HRESULT ResolveIt(HWND hwnd, LPCSTR lpszLinkFile, LPWSTR lpszPath, int iPathBufferSize)


and it could then be used with something like .
 
 
ResolveIt(NULL, L all+"\\Desktop\\Pidgin.lnk",shell, 250);



or maybe I could just convert it right to a LPCSTR but you get the idea?



Mar 5, 2012 at 2:31am
Disch (13742)
Every part of the array wchat_t's of path HAS a number value though...Why?


It has to. Memory has to hold something, you can't have "empty" memory.

I would think it would end with that null character?


It does. The null character marks the end of the string. However if the array is larger you'll have "garbage" that fills the rest of the array.

An alternative to your example:

1
2
3
4
 
for (int i = 0; path[i] != 0; i++)
{
   cout << path[i] << endl;
}


Rather than printing the whole array, you print until you find the null. This is effectively what every function that takes a C-style string does. They all look for that null. That's how they know where to stop processing.


So to convert it to a standard string I concocted


There are lots of ways to do this, but I try to avoid it. It just adds a whole new layer of complexity that you don't need.

I recommend you either work entirely in wide characters, or you don't use wide characters at all. Having to go back and forth between the two is just extra work.

Converting a wchar_t buffer to a wide string is easy:

1
2
3
 
wchar_t whatever[100] = L"example";

wstring foo = whatever; // nice 'n' easy
Mar 5, 2012 at 5:48am
adn258 (73)
Thanks Disch. Phew! This stuff is confusing. Will the thing is I have to use the path USERPROFILE for another part of my program which takes a LPCSTR so how could you possibly just use all wide characters or for that matter not use w_chart when I need to use that string of userpath or it's required by the function?

Thanks in advance
Last edited on Mar 5, 2012 at 5:57am
Mar 5, 2012 at 9:08am
adn258 (73)
I've almost got this the only thing I don't get is how to not use wchar_t etc. when windows functions call for them to be used and I will later need to use those values?
Mar 5, 2012 at 3:44pm
Disch (13742)
Will the thing is I have to use the path USERPROFILE for another part of my program which takes a LPCSTR


Can you change that one thing to use a LPCWSTR? That would be the easiest/best solution here.

the only thing I don't get is how to not use wchar_t


WinAPI funcitons have 3 forms: One which takes 'char' strings (LPCSTR), one which takes wchar_t strings (LPCWSTR) and one which takes TCHAR strings (LPCTSTR). The functions all follow the same naming scheme.

Example:
MessageBox() <- takes LPCTSTR (TCHAR)
MessageBoxW() <- takes LPCWSTR (wchar_t)
MessageBoxA() <- takes LPCSTR (char)

My advise is pick one and use it everywhere.

Note that I've heard rumors that the 'A'/char functions are being deprecated, but I haven't confirmed that and I'm not sure if I believe it.


---------------------------

As for converting between wchar_t and char, it's not really as simple as a straight copy. 'char' isn't bit enough to hold all the same information as 'wchar_t', so if you just do a normal copy from wchar_t to char, you'll lose information. This doesn't matter for plain ASCII strings, so you might not notice (but if you are only dealing with ASCII strings, then you don't need to use wchar_t anyway)


WinAPI treats wchar_t strings as Unicode (UTF-16) encoding. But it can treat char strings any number of ways... by assigning them what's called a "code page". The lower 128 characters are always the same ASCII characters, but the upper 128 chars can change depending on what the code page is set to. If you want your char strings to be able to hold any and all characters, you'd want to set the code page to UTF-8.

Converting UTF-16 to UTF-8 and vice versa isn't exactly trivial. Fortunately WinAPI offers some helper functions for it:

http://msdn.microsoft.com/en-us/library/windows/desktop/dd319072%28v=vs.85%29.aspx <- utf-8 to utf-16
http://msdn.microsoft.com/en-us/library/windows/desktop/dd374130%28v=vs.85%29.aspx <- utf-16 to utf-8

Example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
wchar_t utf16[300] = L"A wide / UTF-16 string";
char utf8[300];

// convert the UTF-16 string to a UTF-8 string
WideCharToMulitByte(
   CP_UTF8,   // we want UTF-8 encoding
   0,    // no other flags
   utf16,  // the source string
   -1,  // source string is null terminated, so we give -1 for the size
   utf8,  // destination string
   300,  // size of destination buffer
   NULL,NULL); // last 2 params are useless for UTF conversions


// now utf16 has been copied to utf8.  To convert back:

MultiByteToWideChar(
   CP_UTF8,  // src string is UTF8
   0,  // no other flags
   utf8,  // source string
   -1,  // null terminated, so no need to give a size
   utf16, // dest string
   300 );  // dest size
Last edited on Mar 5, 2012 at 3:45pm
Mar 5, 2012 at 5:06pm
adn258 (73)
Thanks Disch; Well according to MSDN I thought you had to use for example for resolving shortcuts a window handle LPCstr to the link file and then this outputs WHAT the LINK LINKS TO via the LPWSTR buffer of memory and stores that within there.
1
2
3
4
5
6
7
8
9
10
 

HRESULT ResolveIt(HWND hwnd, LPCSTR lpszLinkFile, LPWSTR lpszPath, int iPathBufferSize) 


wchar_t bn[250];

ResolveIt(NULL, "C:\\Users\\austin\\Desktop\\Pidgin.lnk",bn,250);

wcout << bn << endl;


are you saying that I could get away with changing the output of LPWSTR lpszPath to like an LPCSTR??
Thanks for your help friend
Mar 5, 2012 at 5:53pm
Disch (13742)
Well the "ResolveIt" function is yours, right? So you can change it to be however you want.

It looks like you just copy/pasted the code from this page to get it:

http://msdn.microsoft.com/en-us/library/windows/desktop/bb776891(v=vs.85).aspx

Anyway... you'll notice that the LPCSTR parameter is used in exactly one place in that function: it's being passed to MultiByteToWideChar to expand it to a LPCWSTR. So if you want to get rid of the char string and use only wchar_t strings, all you have to do is get rid of that MultiByteToWideChar call and use the passed string directly.

IE, change this:

1
2
3
4
5
6
7
8
9
10
11
12
13
 
HRESULT ResolveIt(HWND hwnd, LPCSTR lpszLinkFile, LPWSTR lpszPath, int iPathBufferSize) 
{
//...
            WCHAR wsz[MAX_PATH]; 
 
            // Ensure that the string is Unicode. 
            MultiByteToWideChar(CP_ACP, 0, lpszLinkFile, -1, wsz, MAX_PATH); 
 
            // Add code here to check return value from MultiByteWideChar 
            // for success.
 
            // Load the shortcut. 
            hres = ppf->Load(wsz, STGM_READ);


To this...
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
HRESULT ResolveIt(HWND hwnd, LPCWSTR lpszLinkFile, LPWSTR lpszPath, int iPathBufferSize)  //<- change lpszLinkFile to wide string
{
//...
            // WCHAR wsz[MAX_PATH];  <- no need for this any more
 
            // Ensure that the string is Unicode. 
            // MultiByteToWideChar(CP_ACP, 0, lpszLinkFile, -1, wsz, MAX_PATH); <- get rid of this also
 
            // Add code here to check return value from MultiByteWideChar 
            // for success.
 
            // Load the shortcut. 
            // hres = ppf->Load(wsz, STGM_READ);  <- don't use wsz
            hres = ppf->Load(lpszLinkFile, STGM_READ);  // <- instead... use lpszLinkFile directly
Mar 6, 2012 at 1:14am
adn258 (73)
Disch yes in this case I did just use their example code. I generally understand the code but I didn't think this was legal because in the example function at the top on MSDN it shows (can't remember where) it shows how to use a function showing like LPCWSTR etc. and in this it specifically shows LPWSTR but do you have to use exactly what it shows you? Can you sub it with like and LPCSTR I really hate this wide character stuff. I will use what you told me above and try that . I really appreciate your help.
Mar 6, 2012 at 4:09am
adn258 (73)
Disch...I think maybe we are misunderstanding one and other sorry. It's working fine the code and I changed it how you said and it still works fine. My problem is that when you get a widecharecter which is says must be stored in a Wchar buffer on msdn you can get the path that the shortcut links too using ResolveIt but it's just very hard to work with at that point for use in other parts of the program.

This also goes for even simpler functions like ExpandEnvironmentStrings(
It looks like windows makes you use a wide character for the output?
Last edited on Mar 6, 2012 at 4:40am
Mar 6, 2012 at 5:59am
Disch (13742)
It looks like windows makes you use a wide character for the output?


Nope. It simply gives you the option.

Going back to my previous example:

I wrote:

WinAPI funcitons have 3 forms: One which takes 'char' strings (LPCSTR), one which takes wchar_t strings (LPCWSTR) and one which takes TCHAR strings (LPCTSTR). The functions all follow the same naming scheme.

Example:
MessageBox() <- takes LPCTSTR (TCHAR)
MessageBoxW() <- takes LPCWSTR (wchar_t)
MessageBoxA() <- takes LPCSTR (char)


This is true for [as far as I know] ALL WinAPI functions, including ExpandEnvironmentStrings.

If you don't want to deal with wide characters, then don't. Use regular chars and simply put a 'A' after function/struct names (ie: ExpandEnvironmentStringsA)
Mar 6, 2012 at 7:16am
adn258 (73)
Thanks Disch...I'm just beginning win32 api programming so this makes sense now. That's the part that was really getting me. I'm still somewhat new to this and I learned a lot. I was starting to do nutty things like 

1
2
3
4
5
6
7
8
9
10
 
//array for profile path
wchar_t path[250];
char* UserPath= new char[250];
ExpandEnvironmentStringsW(L"%USERPROFILE%",path,250);
WideCharToMultiByte(CP_ACP, WC_NO_BEST_FIT_CHARS, path, -1, UserPath, 250, NULL, NULL);
//string for holding the user path
string sUserPath = UserPath;
//free memory
delete UserPath;
cout << sUserPath << endl;


Which also worked but that seems ridiculous to me. Basically turning widechar to multibyte. I know I keep asking stuff but is there any advantage to that above? That would seem to use more memory if you just needed a standard string in the end and you could have just used ExpandEnvironmentStringsA( ???
Last edited on Mar 6, 2012 at 7:23am
Mar 6, 2012 at 2:27pm
Disch (13742)
1
2
3
4
 
char* UserPath= new char[250];
//...
//delete UserPath;  // <- wrong, memory leak
delete[] UserPath;  // <- correct.  new[] -> delete[]



is there any advantage to that above?


Only advantage I can think of is that you can explicitly state which code page you want to use in the conversion. If you don't care which code page is used (ie: if you only have ASCII text), then no, there's no other advantage AFAIK.
Mar 7, 2012 at 4:03am
modoran (2077)
Windows uses widechar internally, so using multibyte in your code actually makes windows to convert behind the scenes, using more memory and cpu time.

Always use wchar_t.
Mar 7, 2012 at 5:26am
Disch (13742)
To clarify, I didn't mean to say that there was no advantage to wchar_t. I prefer to use it myself when I'm in WinAPI (Unicode FTW).

What I meant to say was that there's no advantage to using ExpandEnvironmentStringsW + WideCharToMultiByte vs. just using ExpandEnvironmentStringsA.


I agree that wchar_t should be preferred, but IMO it's more important to pick one and run with it. Halfass converting back and forth between the two is pointless.
Mar 7, 2012 at 8:15am
adn258 (73)
Disch modoran Thanks for all of your help you cleared that all up for me now phew and you guys are brilliant programmers...

Last question on this seriously I'm just trying to get this all out on ONE TOPIC. So when dealing with memory especially large amounts of memory using the example above isn't it "generally" better to dynamically allocate and then delete that memory vs just using static memory like
char UserPath[250]; ? This makes sense because as best as I know this memory is temporarily stored used and then deleted whilst static memory won't go away correct? Meaning in the long run your program will run slower if you don't allocate and delete large memory sums/buffers ?

Mar 7, 2012 at 4:07pm
Disch (13742)
If the memory is local to a function it is placed on the stack. Stack space costs absolutely nothing because it's all preallocated at the start of the program and freed at program shutdown. You end up using memory that has already been preallocated and remains allocated even if you don't use it.

So no -- it's better to put it on the stack.

The only exception is if the data is so large that it consumes all your stack space -- in which case you risk running out of stack space which will crash your program.

Data has to be really big for this to happen, though... or you have to chain several functions together which consume a lot of stack space.
Mar 8, 2012 at 8:01am
adn258 (73)
So dynamically allocating memory on the heap I assume is better in times when the user will enter a user defined amount of something like numbers etc? Is this When is it better to dynamically allocate memory like that? Thanks again Disch but you've explained a lot and I am learning a lot from you.
Last edited on Mar 8, 2012 at 8:03am
Mar 9, 2012 at 6:40pm
Disch (13742)
Basically, yeah. That's a simplified perspective but you have the right idea.
Topic archived. No new replies allowed.