Hello all.  I am having a problem with GetOpenFilename.  My program runs fine by itself.  The problem happens when the open file dialog comes up.  It will let me open a few files (random number) but when I go to open a new one my program crashes.  I looked at it in ollydbg and it always crashes in shell32.dll.  All I have to do is open the file open dialog and move around a bit and it will crash.  My code for the open dialog is in another asm file as follows:

.code
fileopen proc
mov ofn.lStructSize, SIZEOF ofn
mov ofn.hWndOwner, NULL
push hInstance
pop ofn.hInstance
mov ofn.lpstrFilter, offset ofnFilter
mov ofn.lpstrFile, offset ofnFile
mov ofn.nMaxFile, SIZEOF ofnFile
mov ofn.lpstrFileTitle, offset ofnFileTitle
mov ofn.nMaxFileTitle, SIZEOF ofnFileTitle
mov ofn.lpstrTitle, offset ofnTitle
mov ofn.Flags, OFN_EXPLORER or OFN_FILEMUSTEXIST or \
              OFN_LONGNAMES or OFN_HIDEREADONLY


invoke GetOpenFileName, addr ofn
ret
fileopen endp


I really don't know what is going on.  Hopefully someone will know. :)
Posted on 2005-09-03 20:56:24 by Desp
Make sure ofnFile is zeroed when calling GetOpenFileName. Do a

mov dword ptr , 0 before your call.
Posted on 2005-09-03 21:14:59 by JimmyClif
It appears to be elsewhere.  I tried that and it didn't make a difference.  Thanks for your reply.
Posted on 2005-09-03 21:22:18 by Desp
Is the openfilestructure aligned to a 4-byte boundary?
How is ofnFile and ofnFileTitle declared?

And as jimmy said, always zero out the first byte (or dword, if you want) of ofnFile and ofnTitle before calling GetOpenFileName.

Posted on 2005-09-03 22:45:37 by f0dder
Desp,

This code has been working for years reliably.


    .data
      szFileName    db 260 dup(0)
      ofn          OPENFILENAME <>  ; structure

    .code

; ########################################################################

GetFileName proc hParent:DWORD,lpTitle:DWORD,lpFilter:DWORD

    mov ofn.lStructSize,        sizeof OPENFILENAME
    m2m ofn.hWndOwner,          hParent
    m2m ofn.hInstance,          hInstance
    m2m ofn.lpstrFilter,        lpFilter
    m2m ofn.lpstrFile,          offset szFileName
    mov ofn.nMaxFile,          sizeof szFileName
    m2m ofn.lpstrTitle,        lpTitle
    mov ofn.Flags,              OFN_EXPLORER or OFN_FILEMUSTEXIST or \
                                OFN_LONGNAMES

    invoke GetOpenFileName,ADDR ofn

    ret

GetFileName endp

; #########################################################################
Posted on 2005-09-03 23:08:25 by hutch--
hutch, have you ever called that code a bunch of times in a row? Not zeroing the string has been a problem for me on some windows versions.

Also, you should use MAX_PATH+1 (261) rather than just "260" for filename length - remember that you need space for the zero terminator.
Posted on 2005-09-03 23:13:56 by f0dder
This data (and the code section I posted earlier) are in the separate asm file:

.data
align 4
ofn OPENFILENAME <>
ofnFilter    db "All files(*.*),0,"*.*",0,0
ofnTitle    db "Open",0
ofnFile      db 261 dup (?)
ofnFileTitle db 261 dup (?)


Again this is in another .asm file and not the main one.  The problem is I can open the dialog box and open a file but if I try to do it too many times it crashes when the Open File box is displayed and I move around in it.  It always crashes in shell32. Also I am zeroing the file and filetitle before I call.
Posted on 2005-09-03 23:53:08 by Desp
And as jimmy said, always zero out the first byte (or dword, if you want) of ofnFile and ofnTitle before calling GetOpenFileName.


This is not absolutely true, if you have the fully qualified path to a file in this buffer the dialog will initialize with that file selected. It is very useful for instance when you wish to default to the top MRU in the dialog. I have pretty much always used this...

DATA SECTION
szFilterStr DB "Executable files (*.exe,*.dll,*.ocx,*.cpl)",0,"*.exe;*.dll;*.ocx;*.cpl",0,0
szDefExt DB "EXE",0
szOpenCaption DB "Open file",0
ofrFilePath DB MAX_PATH+1 DUP (?)

CODE SECTION
GetFile FRAME hInstance,hDlg
LOCAL ofn :OPENFILENAME
LOCAL ofrFileTitle[64] :B

mov D,SIZEOF OPENFILENAME
mov eax,
mov ,eax
mov eax,
mov D,NULL
mov ,OFFSET szFilterStr
mov D,NULL
mov D,NULL
mov D,1
mov eax,OFFSET ofrFilePath
mov D,0
mov D,OFFSET ofrFilePath
mov D,MAX_PATH
mov D,NULL
mov D,NULL
mov D,NULL
mov ,OFFSET szOpenCaption
mov D,OFN_FILEMUSTEXIST + OFN_NONETWORKBUTTON + OFN_EXPLORER + OFN_HIDEREADONLY
mov D,NULL
mov D,NULL
mov ,OFFSET szDefExt
mov D,NULL

invoke GetOpenFileName,ADDR ofn
ret
ENDF
Posted on 2005-09-04 00:22:28 by donkey
Ok I have narrowed down the problem.  I tired my program out on another computer also XP, and it works fine.  What could be causing the crash?  I ran a system file check to see if anything was wrong but that didn't help.  I don't think I have a virus or anything...  btw, it crashes on my laptop (XP sp2) but not on the desktop (XP sp1).  What could be causing this annoying error?

edit: On my desktop where it doesn't crash the file open box is in the middle of the screen yet on my laptop it is in the top left.  How can I get it to always be in the center?
Posted on 2005-09-04 00:25:57 by Desp
Mine works on XPSP2 just fine, I would seriously consider setting all of the elements in the structure just in case GetOpenFileName is writing back data to one, that would explain why it takes a few times to appear. By just setting a few of the elements you are allowing the others to contain unknown data. Also you may have a buffer over-run that is writing into the structure at some point and "trashing" data.
Posted on 2005-09-04 00:30:53 by donkey

This is not absolutely true, if you have the fully qualified path to a file in this buffer the dialog will initialize with that file selected. It is very useful for instance when you wish to default to the top MRU in the dialog.

Yeah, while driving home just a few minutes ago, I was pondering why not zeroing the strings would cause problems, and came to the conclusion that (because of the behaviour you just mentioned), local strings could end up overflowing either some internal user32 buffer (if strcpy is used), or read from inacessible stack memory (if the stack doesn't contain a zero-byte somewhere).

Hutch's version doesn't have this problem since he uses global data, but it of course has the problem of 1-too-small buffers, and not being threadsafe.

Desp, have you tried tracing your program in in OllyDbg or some other debugger, to narrow down exactly what function causes the crash?
Posted on 2005-09-04 01:23:43 by f0dder
Yes, I have tried looking at it in olly.  It always generates an exception in shell32.dll.  And my other question: how can I center the GetOpenFilename box?
Posted on 2005-09-04 01:27:19 by Desp

Yes, I have tried looking at it in olly.  It always generates an exception in shell32.dll. 

At what point, though? When stepping over GetOpenFileName, or some other call?
Posted on 2005-09-04 01:31:17 by f0dder
In the GetOpenFilename call.  All I have to do is start my program, click open file, the open file dialog shows up and I hit cancel.  I open it a bunch of times and click cancel until after a random number of opens it crashes when the open file box is open.
Posted on 2005-09-04 02:03:06 by Desp

In the GetOpenFilename call.  All I have to do is start my program, click open file, the open file dialog shows up and I hit cancel.  I open it a bunch of times and click cancel until after a random number of opens it crashes when the open file box is open.


Have you tried to explicitly set all of the members of the structure, even those that you would normally not set or use ? As I said it could be  a buffer overrun into the structure that is passing some innapropriate data to the API call. The problem is definitely in your program somewhere and not in the API as it is a pretty high traffic API and any problems would have been noted some time ago. Try moving the struct and buffers to global memory and see if the problem clears up, it may be nothing more than stack pollution that is causing the problem. I would especially suggest this as you are placing the structure on the stack without resetting many of it's members to zero.
Posted on 2005-09-04 07:53:39 by donkey
I have set all the members of the structure.  I can't understand why it will work on one computer and not the other.  I tried compiling the exact same code one the computer that works and everything runs fine.  I even copied it back to the other computer to see if it would work.  Same thing happens. This leads me to believe that it isn't a compiler problem but for some reason it is crashing.  Could it be there is a leak but the amount of memory on the desktop is bigger so it would take longer to overflow?  I really can't figure this out.
Posted on 2005-09-04 08:02:44 by Desp
hi all ...
i also get problem with GetOpenFileName function ... if ofn struct is declared as global varialbe -->  no problem, but if it is declared as local variable, it will crash .
could someone explain this ??? .
Posted on 2005-09-04 08:49:13 by secmask
Desp,

Just try setting the structure length member.

    mov ofn.lStructSize,        sizeof OPENFILENAME


Normally when there is a leading structure size member, if you don't set it to at least the structure size, you get unpredictable results. It means that the API needs this information internally.

If you want to set te structure up as a LOCAL, you will have to zero fill it unless you want to manually fill out every member.
Posted on 2005-09-04 09:00:17 by hutch--
Hutch,

I have moved the size of the structure into ofn.lStructSize.  My program works fine on my other computer but not on my laptop.  This only crashes with the file open dialog of my program and not any others.  I can't understand why the same code will work on my other computer but not this one.
Posted on 2005-09-04 09:11:35 by Desp
Desp, I have just pasted your two snippets (function, and data) into a skeleton program.

First observation: The program wouldn't compile because of a missing quotation mark in your declaration of ofnFilter:

ofnFilter    db "All files(*.*),0,"*.*",0,0


It did compile after adding the quotation mark:

ofnFilter    db "All files(*.*)",0,"*.*",0,0


I don't know where that quotation mark got lost, but maybe you should double-check if you are actually including the file that you intend to include.

Second observation: On an XP-SP2 machine, I could not reproduce the problem that you described, despite seriously trying. Could you post some minimal code, or maybe a stripped-down executable that demonstrates the undesired behaviour?

Regards

    Frank
Posted on 2005-09-04 12:55:43 by Frank