See if you can find the problem here:
; First, zero out entire structure:
PUSH EDI
LEA EDI, ofn
MOV ECX, SIZEOF OPENFILENAME
MOV ofn.lStructSize, ECX ;Stuff length while we have it.
XOR EAX, EAX
REP STOSB ;Zero it out.
POP EDI
This one survived about a half-hour of debugging, head scratching and swearing ...
("ofn" is the structure used by GetOpenFileName() )
I post this in hopes that others won't make this same mistake.
Better?
LOCAL ofn:OPENFILENAME
push edi
lea edi, ofn
push OPENFILENAME
pop eax
stosd
xchg eax, ecx
sub ecx, 4
xor eax, eax
rep stosb ;zero it out.
pop edi
:red
lea edi, ofn
:P
mov edi,offset ofn
szSaveAs db 'Save As...',0
szFileTypes db 'Text Files',0,'*.txt',0,'All Files',0,'*.*',0,0
.CODE
;EAX = filename buffer address
;ECX = 0
;EDX = filename buffer length
push ecx ;lpTemplateName
push ecx ;lpfnHook
push ecx ;lCustData
push offset szFileTypes+13 ;lpstrDefExt - 'txt',0
push ecx ;nFileOffset/nFileExtension
push OFN_LONGNAMES or OFN_HIDEREADONLY or OFN_OVERWRITEPROMPT ;Flags or OFN_ENABLEHOOK or OFN_EXPLORER
push offset szSaveAs ;lpstrTitle
push ecx ;lpstrInitialDir
push ecx ;nMaxFileTitle
push ecx ;lpstrFileTitle
push edx ;nMaxFile
push eax ;lpstrFile
push 1 ;nFilterIndex
push ecx ;nMaxCustFilter
push ecx ;lpstrCustomFilter
push offset szFileTypes ;lpstrFilter
push ecx ;hInstance
push hWnd ;hwndOwner
push 76 ;lStructSize
INVOKE GetSaveFileName,esp
add esp,76 ;sizeof OPENFILENAME
; First, zero out entire structure
; Then, fill in ofn.lStructSize
edit: jj forgot something?
it may not be a local - if it is, then use LEA EDI,ofn
push edi
mov edi,offset ofn
push sizeof OPENFILENAME
push (sizeof OPENFILENAME-4)/4
pop ecx
pop eax
stosd
xor eax,eax
rep stosd ;zero out the rest of it
pop edi
of course, we shouldn't do this in the campus, but it's fun :bg
and, NoCforMe isn't a real n00b, anyways
Quote from: sinsi on October 11, 2011, 11:34:05 AM
; First, zero out entire structure
; Then, fill in ofn.lStructSize
edit: jj forgot something?
No :bg
PUSH EDI
LEA EDI, ofn
MOV ECX, SIZEOF OPENFILENAME
MOV ofn.lStructSize, ECX ;Stuff length while we have it.
XOR EAX, EAX
REP STOSB ;Zero it out.
POP EDI
MOV ofn.lStructSize, SIZEOF OPENFILENAME; :-)
@drizz: as it stands, 36 bytes; with small changes...
LOCAL ofn:OPENFILENAME
push edi
lea edi, ofn
mov ecx, sizeof OPENFILENAME
;MOV ofn.lStructSize, ECX ;Stuff length while we have it.
xor eax, eax
rep stosb ;zero it out.
pop edi
m2m ofn.lStructSize, sizeof OPENFILENAME;
ret
... down to 31, still 2 more than mine ;)
mov esi, alloc(SIZE OPENFILENAME)
mov [esi].OPENFILENAME.lStructSize, SIZE OPENFILENAME;
Smaller than 31 bytes?
:lol I know technically not but size is relative
Quote from: oex on October 11, 2011, 12:47:21 PM
Smaller than 31 bytes?
Sorry, it's 32 bytes ;-)
MyTesto proc arg1, arg2
push esi
mov esi, alloc(SIZE OPENFILENAME)
mov [esi].OPENFILENAME.lStructSize, SIZE OPENFILENAME
; ...
free esi
pop esi
ret
MyTesto endp
Quote from: jj2007 on October 11, 2011, 12:55:31 PM
Quote from: oex on October 11, 2011, 12:47:21 PM
Smaller than 31 bytes?
Sorry, it's 32 bytes ;-)
:lol.... Well I beat the almighty Drizz.... I am not ashamed.... And my code is pretty neat and tidy :lol
Quote from: dedndave on October 11, 2011, 11:37:28 AM
it may not be a local - if it is, then use LEA EDI,ofn
push edi
mov edi,offset ofn
push sizeof OPENFILENAME
push (sizeof OPENFILENAME-4)/4
pop ecx
pop eax
stosd
xor eax,eax
rep stosd ;zero out the rest of it
pop edi
18 bytes if ofn is global
16 bytes if ofn is local
Dave,
It's 28 bytes with frame etc, but still: 1 byte shorter than my version - you win :cheekygreen:
:lol
i dunno how i miscounted the bytes, though
Quote from: dedndave on October 11, 2011, 01:42:03 PM
:lol
i dunno how i miscounted the bytes, though
it's the overhead.
MyTestdave_s: ; 28
MyTestdave proc arg1, arg2
LOCAL ofn:OPENFILENAME
dave_s:: ; 16
push edi
lea edi, ofn
push sizeof OPENFILENAME
push (sizeof OPENFILENAME-4)/4
pop ecx
pop eax
stosd
xor eax,eax
rep stosd ;zero out the rest of it
pop edi
dave_endp::
ret
MyTestdave endp
MyTestdave_endp:
ok - i wasn't counting the code i didn't write :P
Guys, guys, guys: You're completely missing the point. You can't see the forest for the trees.
The code I posted doesn't work correctly. There's a glaring error in it (hence the "rookie mistake"). I finally fixed it, and I posted this as a possible help to others.
It wasn't intended as an exercise on how to generate the absolute smallest code.
I gave you a problem with changing a tire on a car and you all responded with stuff on quantum mechanics. Sheesh.
So class, can anyone tell me what the problem was with my code? (BTW, the structure is a LOCAL, but it doesn't make any difference.)
Quote from: NoCforMe on October 11, 2011, 06:14:39 PM
It wasn't intended as an exercise on how to generate the absolute smallest code.
So class, can anyone tell me what the problem was with my code?
It wasn't small enough?
jj, I didn't realize it was a pissing competition. :red
Quote from: NoCforMe on October 11, 2011, 06:14:39 PM
Guys, guys, guys: You're completely missing the point. You can't see the forest for the trees.
The code I posted doesn't work correctly. There's a glaring error in it (hence the "rookie mistake"). I finally fixed it, and I posted this as a possible help to others.
It wasn't intended as an exercise on how to generate the absolute smallest code.
I gave you a problem with changing a tire on a car and you all responded with stuff on quantum mechanics. Sheesh.
So class, can anyone tell me what the problem was with my code? (BTW, the structure is a LOCAL, but it doesn't make any difference.)
:lol
; First, zero out entire structure:
PUSH EDI
LEA EDI, ofn
MOV ECX, SIZEOF OPENFILENAME
MOV ofn.lStructSize, ECX ;Stuff length while we have it.
XOR EAX, EAX
REP STOSB ;Zero it out.
POP EDIthe problem that i see is that you set the lStructSize member, then overwrite it with 0(s)
that probably generates an ERROR_INVALID_PARAMETER or ERROR_INVALID_DATA error
Not even close, and no cigar.
Wow, maybe it wasn't that glaring an error after all. As I said, I was banging my head against the keyboard for a good half hour before I spotted it.
Hint: the error I got back from GetOpenFileName() was CDERR_STRUCTSIZE. (That's too big a hint, actually. And you have to call CommDlgExtendedError() to retrieve the error code.)
@Dave: Bingo! You got it. (Actual error is shown above.)
Quote from: drizz on October 11, 2011, 06:23:32 PM
jj, I didn't realize it was a pissing competition. :red
Quote from: dedndave on October 11, 2011, 11:37:28 AM
of course, we shouldn't do this in the campus, but it's fun :bg
OK, maybe you are right that we shouldn't play such games in the Campus. The Forum rules clearly say it's a humour-free zone :red
Therefore, for all those who want to learn assembly in the Campus (OP code):
MOV ofn.lStructSize, ECX ;Stuff length while we have it. <<< that's ok
XOR EAX, EAX
LEA EDI, ofn ; that points to the same memory as ofn.lStructSize
REP STOSB ; and here we mercilessly overwrite the ecx value
Still, those who want to write correct and elegant code should study Dave's example. Even in the Campus :bg
Quotethe problem that i see is that you set the lStructSize member, then overwrite it with 0(s)
i thought that's what i said :P
Yes, the co-winners are jj and Dave. You may collect your prize (a brand-new shrink-wrapped copy of DOS 3.1) as you leave.
By the way, I vote in favor of fun, at all times. We can do things the correct way, or we can do things the fun way ...
Quote from: NoCforMe on October 11, 2011, 06:14:39 PM
Guys, guys, guys: You're completely missing the point. You can't see the forest for the trees.
The code I posted doesn't work correctly.
We surely saw the forest. All examples posted work correctly, and yes everybody saw the "glaring mistake" but nobody in here smokes cigars :wink
... and as we all know, the direction flag is a constant :wink
seeing as we are on the subject of the OPENFILENAME structure size :bg
if you look on MSDN, it is defined as
typedef struct tagOFN {
DWORD lStructSize;
HWND hwndOwner;
HINSTANCE hInstance;
LPCTSTR lpstrFilter;
LPTSTR lpstrCustomFilter;
DWORD nMaxCustFilter;
DWORD nFilterIndex;
LPTSTR lpstrFile;
DWORD nMaxFile;
LPTSTR lpstrFileTitle;
DWORD nMaxFileTitle;
LPCTSTR lpstrInitialDir;
LPCTSTR lpstrTitle;
DWORD Flags;
WORD nFileOffset;
WORD nFileExtension;
LPCTSTR lpstrDefExt;
LPARAM lCustData;
LPOFNHOOKPROC lpfnHook;
LPCTSTR lpTemplateName;
#if (_WIN32_WINNT >= 0x0500)
void *pvReserved;
DWORD dwReserved;
DWORD FlagsEx;
#endif
} OPENFILENAME, *LPOPENFILENAME;
the new members apply to everything windows 2000 and above
however, the windows.inc file defines the structure without them
if you refer to this post, you will see that i hard-code the size value so that it assembles the same if windows.inc is updated
http://www.masm32.com/board/index.php?topic=17525.msg147288#msg147288
Quote from: qWord on October 11, 2011, 06:42:14 PM
... and as we all know, the direction flag is a constant :wink
Good point, and I thought of that as I was coding this: what if someone leaves the direction flag set?
Made me think that, while I've never seen the innards of Windows code (I mean the OS), I imagine every place that uses string moves/stores/scans probably does something like this:
pushf
cld
(operation involving STOSx, SCASx, MOVSx, CMPSx}
popf
in order to "bulletproof" it, don't you think?
By default, the direction flag is cleared. If you have to set it (rare case), it's your duty to clear it on exit. You would use cld, not a slow pushfd/popfd pair.
it's part of the ABI definition - that the DF should remain cleared
in fact, some API functions will hang or have other problems if called with it set - some get really slow
as a side note, instructions that set or clear this flag (POPFD, CLD, STD) execute surprisingly slow
if i am interested in speed, i will sometimes test the flag bit, executing CLD only if it is set
Ah, I see: yet another way to crash Windoze:
STD--> Boom!
(And of course I mean STD without a corresponding CLD.)
Quote from: NoCforMe on October 11, 2011, 07:13:44 PM
Ah, I see: yet another way to crash Windoze
It won't crash Windows, but it may cause your app to crash.