GetOpenFileName fails

[Landsfiskalen]

Hi!
Ok, so I'm writing a small GUI for mkvpropedit.exe (part of the MKVToolnix suite). Mkvpropedit.exe can be used to add chapters to MKV files. I have a bunch of ripped Blu-Rays that I've forgotten to include chapter information to, so decided to write this GUI to make it easy to apply.

Basically I've got a window set up with two inputs (chapter file and MKV file), two browse buttons and an "Apply"-button. Everything works great, I can click on the browse buttons and select chapter file and MKV file, and then apply the chapter file to the MKV by clicking the "Apply"-button.

But if I try to click on the browse buttons again, the open file dialog doesn't show. GetOpenFileName just returns 0 (ie. an error occured). I can't for the Life of me figure out what's wrong. Any help would be greatly appreciated.

Code Select Expand


WindowProcedure proc private Uses ebx edi esi hWnd:HWND, uMsg:ULONG, wParam:WPARAM, lParam:LPARAM
    local mbp:MSGBOXPARAMS, sui:STARTUPINFO, pi: PROCESS_INFORMATION, dwLastErr: dword, szLastError[1024]: byte
.if uMsg == WM_CREATE
xor eax, eax
ret
.elseif uMsg == WM_DESTROY
invoke DeleteObject, hChapterLabel
invoke DeleteObject, hMKVLabel
invoke DeleteObject, hChapterEdit
invoke DeleteObject, hMKVEdit
invoke DeleteObject, hChapterButton
invoke DeleteObject, hMKVButton
invoke DeleteObject, hApplyButton
invoke DeleteObject, hDefaultFont
invoke PostQuitMessage, 0
xor eax, eax
ret
.elseif uMsg==WM_PAINT

.elseif uMsg == WM_COMMAND
mov eax, lParam
.if eax == hChapterButton
invoke RtlZeroMemory, addr ofn, sizeof ofn
        mov ofn.lStructSize, sizeof ofn
            push hWnd
            pop  ofn.hwndOwner
            push hInst
            pop  ofn.hInstance
            mov  ofn.lpstrFilter, offset szChapterFilterString
            mov  ofn.lpstrFile, offset szBuffer
            mov  ofn.nMaxFile,MAX_PATH
            mov  ofn.Flags, OFN_FILEMUSTEXIST or OFN_PATHMUSTEXIST or OFN_LONGNAMES or OFN_EXPLORER or OFN_HIDEREADONLY
            mov  ofn.lpstrTitle, offset szOpenChapterTitle
invoke GetOpenFileName, addr ofn
.if eax == 0
xor eax,eax
            ret
          .endif
          ; We have a chapter file set it in the hChapterEdit
          invoke SendMessage, hChapterEdit, WM_SETTEXT, 0, ofn.lpstrFile
        .elseif eax == hMKVButton
invoke RtlZeroMemory, addr ofn, sizeof ofn
        mov ofn.lStructSize, sizeof ofn
            push hWnd
            pop  ofn.hwndOwner
            push hInst
            pop  ofn.hInstance
            mov  ofn.lpstrFilter, offset szMKVFilterString
            mov  ofn.lpstrFile, offset szBuffer
            mov  ofn.nMaxFile,MAX_PATH
            mov  ofn.Flags, OFN_FILEMUSTEXIST or OFN_PATHMUSTEXIST or OFN_LONGNAMES or OFN_EXPLORER or OFN_HIDEREADONLY
            mov  ofn.lpstrTitle, offset szOpenMKVTitle
invoke GetOpenFileName, addr ofn
.if eax == 0
xor eax,eax
            ret
          .endif
          ; We have a filename set it in the hChapterEdit
          invoke SendMessage, hMKVEdit, WM_SETTEXT, 0, ofn.lpstrFile       
        .elseif eax == hHelpButton
invoke RtlZeroMemory, addr mbp, sizeof mbp
    mov mbp.cbSize, sizeof mbp
    push hWnd
    pop mbp.hwndOwner
    push hInst
    pop mbp.hInstance
    mov mbp.lpszText, offset szHelpText
    mov mbp.lpszCaption, offset szHelp
    mov mbp.dwStyle, MB_USERICON
    mov mbp.lpszIcon, 100
    mov mbp.dwContextHelpId, NULL
    mov mbp.lpfnMsgBoxCallback, NULL
    mov mbp.dwLanguageId, NULL
        invoke MessageBoxIndirect, addr mbp
.elseif eax == hApplyButton
; Check if mkvpropedit.exe is present, then check that we have selected both chapter & mkv file
invoke SendMessage, hChapterEdit, WM_GETTEXTLENGTH, 0, 0
test eax, eax
jz EmptyChapter
invoke SendMessage, hMKVEdit, WM_GETTEXTLENGTH, 0, 0
test eax, eax
jz EmptyMKV
; concat strings
invoke SendMessage, hChapterEdit, WM_GETTEXT, MAX_PATH, Offset szChapterBuffer
invoke SendMessage, hMKVEdit, WM_GETTEXT, MAX_PATH, Offset szMKVBuffer
invoke RtlZeroMemory, addr szBuffer, sizeof szBuffer
invoke lstrcat, addr szBuffer, addr szMKVPropeditCmd
invoke lstrcat, addr szBuffer, addr szChapterBuffer
invoke lstrcat, addr szBuffer, addr szMKVPropeditCmd2
invoke lstrcat, addr szBuffer, addr szMKVBuffer
invoke lstrcat, addr szBuffer, addr szMKVPropeditCmd3
invoke GetModuleFileName, hInst, addr szExebuffer, MAX_PATH
invoke PathRemoveFileSpec, addr szExebuffer
invoke lstrcat, addr szExebuffer, addr szMKVPropeditPrg
invoke RtlZeroMemory, addr sui, sizeof sui
mov sui.cb, sizeof sui
invoke CreateProcess, addr szExebuffer, addr szBuffer, NULL, NULL, NULL, CREATE_NEW_PROCESS_GROUP or NORMAL_PRIORITY_CLASS or CREATE_NO_WINDOW, NULL, NULL, addr sui, addr pi
.if (eax != 0)
invoke MessageBox, hWnd, addr szSuccessStr, addr szHelp, MB_OK or MB_ICONINFORMATION
invoke CloseHandle, pi.hThread
invoke CloseHandle, pi.hProcess
.elseif
    invoke GetLastError
mov dwLastErr,eax
invoke FormatMessage,FORMAT_MESSAGE_FROM_SYSTEM,0,dwLastErr,0,addr szLastError,1024,0
invoke MessageBox, hWnd, addr szLastError, addr szError, MB_OK or MB_ICONERROR
.endif
ret
EmptyChapter:
invoke MessageBox, hWnd, addr szChapterEmpty, addr szError, MB_OK or MB_ICONERROR
ret
EmptyMKV:
invoke MessageBox, hWnd, addr szMKVEmpty, addr szError, MB_OK or MB_ICONERROR
ret
.endif
.endif
invoke DefWindowProc, hWnd, uMsg, wParam, lParam
ret
WindowProcedure endp


[Zen]

Hi, Landsfiskalen,
I read your code, and I googled for: matroska file, because I had no idea what the MKVToolnix suite was. It's hard to tell for absolute certain where your problem is because you don't tell us what all your defines are.
If what you are describing is correct, then we should be looking at the line:

Code Select Expand

uMsg == WM_COMMAND

Is this correct ?
If  GetOpenFileName fails to open the dialog box, I would guess that the message handler is not even being called.

I'd say that your .if, .elseif, .endif block hierarchy is set up wrong. 
(Although it looks as if it MIGHT work, assuming your defines are correlated correctly, and your SendMessage's actually do something.)

...And, read DAVE below,...I'd handle the wparams first and then the lparams,...
Rather that using alot of .if, .elseif, .endif blocks nested within a long .if, .elseif, .endif block, try using a cmp instructions, with a contingent jump based on the outcome to some label. You clearly got lost within the .if, .elseif, .endif blocks. Hell, even DAVE and I got lost in there,...

[dedndave]

it's a little hard to follow the logic of that code - lol

2 things i notice...

1) it's probably simpler to use wParam to sort out WM_COMMAND messages
for a control (like a button), the high word contains the button notification code and the low word contains the ID
for a menu item, the high word is always BN_CLICKED (0)

button notifications
http://msdn.microsoft.com/en-us/library/windows/desktop/ff485887%28v=vs.85%29.aspx
notice that you receive button notifications for things other than clicked   :P

2) create a procedure to create an open file dialog, passing parameters to it
you can then simplify your WndProc code by invoking that procedure
this will make it easier to find bugs

[dedndave]

here's a simple example of using a procedure
notice that the OPENFILENAME structure members are pushed onto the stack in reverse order

Code Select Expand

;place the PROTOtype near the beginning of the source
;typically, this would immediately follow the "standard" includes
;it may be placed into a project include file

OpnFileDlg PROTO :HWND,:LPVOID,:LPSTR


;example use:

        INVOKE  OpnFileDlg,hWnd,offset szChapterFilterString,offset szOpenChapterTitle

;--------------------------------------------------------------------------------

        OPTION  PROLOGUE:None
        OPTION  EPILOGUE:None

OpnFileDlg PROC hWndOwner:HWND,lpFilter:LPVOID,lpszTitle:LPSTR

;Call With: hWndOwner = hWnd of ownder window
;           lpFilter  = address of filter string
;           lpszTitle = address of title string
; 
;  Returns: EAX           = GetOpenFileName call return value
;           ECX           = nFilterIndex
;           EDX low word  = nFileOffset
;           EDX high word = nFileExtension

;----------------------------------

    xor     ecx,ecx
    push    ecx                   ;lpTemplateName
    push    ecx                   ;lpfnHook
    push    ecx                   ;lCustData
    push    ecx                   ;lpstrDefExt
    push    ecx                   ;nFileOffset (low word) and nFileExtension (high word)
    push    OFN_FILEMUSTEXIST or OFN_PATHMUSTEXIST or OFN_LONGNAMES or OFN_EXPLORER or OFN_HIDEREADONLY ;Flags
    push    lpszTitle             ;lpstrTitle
    push    ecx                   ;lpstrInitialDir
    push    ecx                   ;nMaxFileTitle
    push    ecx                   ;lpstrFileTitle
    push    MAX_PATH              ;nMaxFile
    push    offset szBuffer       ;lpstrFile
    push    ecx                   ;nFilterIndex
    push    ecx                   ;nMaxCustFilter
    push    ecx                   ;lpstrCustomFilter
    push    lpFilter              ;lpstrFilter
    push    ecx                   ;hInstance (only needed if template is used)
    push    hWndOwner             ;hwndOwner
    push    76                    ;lStructSize = sizeof OPENFILENAME (old version)
    INVOKE  GetOpenFileName,esp
    mov     ecx,[esp].OPENFILENAME.nFilterIndex
    mov     edx,dword ptr [esp].OPENFILENAME.nFileOffset ;nFileExtension in high word
    add     esp,76                ;sizeof OPENFILENAME (old version)
    ret     12                    ;return and discard 3 dword arguments

OpnFileDlg ENDP

        OPTION  PROLOGUE:PrologueDef
        OPTION  EPILOGUE:EpilogueDef

;--------------------------------------------------------------------------------

EDIT: corrected size issue (line before GetOpenFileName call)

[Zen]

Strange,...DAVE,...reading your code doesn't give me a headache,...

[dedndave]

i'm sure i'll get a lot of flack for building the structure with PUSH - lol
but, it avoids silent crashes due to stack page boundry violations   
and - it's small code, too

[Landsfiskalen]

Gah! Sorry for the weird formatting.  :icon_confused:

Zen>> The message handler is called, and lParam contains the handle to the clicked button. The weird thing is that it works absolutely fine everytime to execute GetOpenFilename, EXCEPT after I've clicked the "Apply"-button. After that it still enters the message handler, it has the correct handle to the button, but GetOpenFilename exits with 0 (ie. error).

I'll try to sort it out using the input from you and dedndave.

Thanks!

[Landsfiskalen]

...

1) it's probably simpler to use wParam to sort out WM_COMMAND messages
for a control (like a button), the high word contains the button notification code and the low word contains the ID
for a menu item, the high word is always BN_CLICKED (0)

button notifications
http://msdn.microsoft.com/en-us/library/windows/desktop/ff485887%28v=vs.85%29.aspx
notice that you receive button notifications for things other than clicked   :P

But isn't that only true if I use a Dialog as main window? I create my window with CreateWindowEx. And if I debug and step through lParam has the handle to the button (also created with CreateWindowEx). I'm confused.  :icon_confused:

[dedndave]

you can use the handle, don't get me wrong

however, each button is also assigned an integer ID number when they are created
you might assign them as 101, 102, 103, and so on
it's easier to compare a register to an immediate constant, than it is to compare it to a stored handle

but, if you compare only the handle, and not the notification code,
you risk executing "button push" code for any and all notifications received from that button

the nice thing is, the value of the BN_CLICKED notification is 0
so, if you load, say, EDX with wParam, the high word is zero when the button is pushed
the low word, then, contains the button ID (which is always less than 65536)

so.....

Code Select Expand

        mov     edx,wParam
        .if edx==101
            ;do stuff for button ID 101 pushed
        .elseif edx==102
            ;do stuff for button ID 102 pushed
        .endif

EDX = 101 means the high word is 0, and you know the notification is BN_CLICKED   
in effect, you made 2 comparisons with one operation
the notification code AND the control ID

[jj2007]

I can't for the Life of me figure out what's wrong.

Nothing is wrong, at least with part one of your code, below in a readable format. The error is somewhere else.

   .if eax == hChapterButton
      invoke RtlZeroMemory, addr ofn, sizeof ofn
      mov ofn.lStructSize, sizeof ofn
      m2m ofn.hwndOwner, hWnd
      m2m ofn.hInstance, hInst
      mov ofn.lpstrFilter, offset szChapterFilterString
      mov ofn.lpstrFile, offset szBuffer
      mov ofn.nMaxFile, MAX_PATH
      mov ofn.Flags, OFN_FILEMUSTEXIST or OFN_PATHMUSTEXIST or OFN_LONGNAMES or OFN_EXPLORER or OFN_HIDEREADONLY
      mov ofn.lpstrTitle, offset szOpenChapterTitle
      invoke GetOpenFileName, addr ofn
      .if eax == 0
         ; xor eax, eax not needed - eax is already zero, right?
         ret
      .endif
      ; We have a chapter file set it in the hChapterEdit
      invoke SendMessage, hChapterEdit, WM_SETTEXT, 0, ofn.lpstrFile
   .elseif eax == hMKVButton
   .endif

[adeyblue]

Chuck in a call to CommDlgExtendedError, it'll give you a broad hint as to what the problem is.

[Zen]

LANDSFISKALEN,
...I had a thought last night as I was falling asleep (weird, huh ?)
I noticed once when I was examining one of my programs for those insidious problems that you can never figure out,...(and, that eventually drive you crazy),...
I had opened the file (compiled from Assembly language with the MASM compiler) in: IDA Pro, freeware version, and I noticed that the MASM compiler generates some interesting code when it encounters an .IF, .ELSEIF, .ENDIF block. It surprised me,...in my case the compiler inserted an OR instruction, and then either a JZ or JNZ, to a label, depending on the context. I suspect that the compiler has a number of these short instruction inserts that it generates as necessary. We've all had this happen at some point,...a complex, nested .IF, .ELSEIF, .ENDIF block that malfunctions at some point. And, if you convert it to a series of CMPs, or TESTs instead of the .IF, .ELSEIF, .ENDIF format, then it seems to work fine. I would open your executable in IDA Pro, and look at what series of instructions the compiler has generated for your Window Proc,...I'll bet you notice the problem section almost immediately.