PerformAction proc

LOCAL Buffer[256]:BYTE

invoke GetDlgItemText, hWind, IDC_EDT1, ADDR Buffer, 255

invoke lstrcat, addr Buffer, OFFSET RegSvr32
invoke lstrcat, OFFSET UnReg, eax
invoke lstrcat, eax, CStr( "\", 0 )

invoke MessageBox, hWind, eax, CStr("DLL File Name and Path"), MB_OK
PerformAction endp

Pardon my ugly coding skills. :D

Here is the problem:
The above PROC works, but to a point. When it is called, it displays the message I want correctly, but only ONE time. If I call this PROC (via a button press) more than 4 times, the program crashes out. It, for some reason, will only display the message I want ONCE. Basically, the message box shows me the path and name of a DLL file that I select using my application by reading the text from the TextBox control I have on my dialog.

Any ideas how I fix this? Been bugging me damn near a week now and I am sure I am missing something simple!

Many thanks,
Posted on 2004-01-20 16:34:27 by The Beginner
The first lstrcat should be lstrcpy, your buffer is not gauranteed to be empty on calling the procedure, it can have garbage left on the stack by other procedures.
Posted on 2004-01-20 17:24:01 by donkey
Shouldn't it work, since he does GetDlgItemText with the buffer as dest?
Posted on 2004-01-20 17:28:50 by f0dder

Shouldn't it work, since he does GetDlgItemText with the buffer as dest?

Geez you're right, forget my post. Eyes are getting old :) Probably just a buffer overrun then.
Posted on 2004-01-20 17:29:56 by donkey
thought about that too, but with a 256byte buffer? :/

- probably something really obvious that'll make me go "DUH!" when somebody else sees it ;)

TB, run your code under OllyDbg or similar and see if you can find something funky?
Posted on 2004-01-20 17:33:24 by f0dder
Hmm... how would I clear the buffer then?
Posted on 2004-01-20 17:33:56 by The Beginner
You shouldn't have to, the GetDlgItemText should take care of that.
Though, to be safe, you might want to set the first byte of the buffer to 0...
Posted on 2004-01-20 17:37:23 by f0dder

Hmm... how would I clear the buffer then?

Just make sure that the UnReg buffer is big enough to hold the two, in the second lstrcat you are writing to that buffer not the one you declared locally. I think it's a buffer overrun myself, but I could be wrong.
Posted on 2004-01-20 17:37:46 by donkey
Oh, I missed that he stores to that other buffer!
Posted on 2004-01-20 17:39:19 by f0dder
Your buffer should be larger then the maximum amount that you are getting from GetDlgItemText (256) at least the size of RegSvr32 + 256 + 1 ( for a NULL terminator)
Posted on 2004-01-20 17:50:02 by ENF
Your first lstrcat concatinates RegSvr32 to the string in the local buffer.
The second lstrcat concatinates the latter with "something" in UnReg.

If that UnReg buffer is not re-initialized before each call to your proc, it may rapidly be overflowed and even result in a program crash trying to write outside the allocated memory.

Remember that the lstrcat proc moves the terminating 0 to the end of the two concatinated strings, making it a single null-terminated string. Any subsequent call to that proc without re-initializing the buffer will simply append another string to it, making it that much longer.

Note that a simple initialization of a buffer only needs its 1st byte set to 0. In your case, you may have some other "prefix" string required for your display. In that case, simply reset the "0" byte where it should be to terminate that prefix.

Posted on 2004-01-20 22:52:20 by Raymond
get dlg -->blah
lstrcat --->blahregsvr32
unreg == null first time -->becomes blahregsvr32
after this it becomes blahregsvr32\ and unreg is blahregsvr32
second time
third time

and so on

but crash why (is your unreg too in stack or you defined it in .data with dup if it is in stack then after the limit is over it will start coruupting the stack if it is in .data the it might be overwriting some required values dunno try it in olly )
Posted on 2004-01-21 11:28:10 by bluffer
invoke lstrcat, OFFSET UnReg, eax

Ok, that is obviously the offending code line.

UnReg is defined in the .data section like this:
UnReg db "Unregister", 0

My problem now is that I need to reset that somehow (I do not know how), or I need to find a better way to write this procedure out.

Any hints? :)

Thanks a lot for the help so far guys. I really appreciate it.
Posted on 2004-01-22 09:23:06 by The Beginner
Haven't tried this but it should work:

PerformAction proc

LOCAL Buffer[256]:BYTE

invoke lstrcpy,ADDR Buffer,OFFSET UnReg
invoke lstrlen,ADDR Buffer
lea eax,[Buffer + eax]
invoke GetDlgItemText, hWind, IDC_EDT1, eax, 255

invoke lstrcat, addr Buffer, OFFSET RegSvr32
invoke lstrcat, ADDR Buffer, CStr( "\", 0 )

invoke MessageBox, hWind, ADDR Buffer, CStr("DLL File Name and Path"), MB_OK
PerformAction endp
Posted on 2004-01-22 09:58:14 by donkey
A simple way to solve your problem without changing too much of your code would be to add the following instruction to re-initialize your "prefix" before you use the UnReg buffer:
mov UnReg[10],0

You must also insure that sufficient space is available in your data section immediately following the declaration of the UnReg variable for whatever strings you intend to append to that "prefix".
UnReg db "Unregister", 0

db xxx dup(?)

That was not too clear in your last post, leaving the impression that you may have been overwriting some of your other data.

Posted on 2004-01-22 12:42:02 by Raymond
Thanks donkey. Your fixes to my code helped. No more crash and no more single instance of the message.

Thanks also to everyone else. You guys are great to help out someone new to this.
Posted on 2004-01-22 13:12:24 by The Beginner