i need help, to complete my first function in masm32 "GetFileName" :)...

[Dasar]

hi all


i wrote this littel function to get the file name from a specific path, this function has 2 operands, the first is the address of the path, the

second is the destination "buffer" which will contain the file name,  for example:

"C:\somedir\SomeFile.exe"

my function supposed to put the "SomeFile.exe" in the destination buffer..

the function is written in this manner

GetFileName PROTO :DWORD, :DWORD

GetFileName proc lpPath: DWORD, dstBuffer: DWORD

=========================================================

i know that maybe there is a function in masm32 does the same mission, but i didn't search for any function, because the main

purpose of writing this function, is the training on write functions in asm...

=========================================================

there are no errors in the syntax of the code, it assembled and linked successfully, but at the run time; and once the program is

exectued,  it crashes


please take a look at the code, and tell me where is the mistake(s), and correct them for me, and if you have a little time, please fill it

with your comments, so i can understand your code easily..

please also tell me if there is an easier and better way to write such funcion.


=========================================================




this is all my code, filled with my comments "these comments written as my understanding"   :

Code Select Expand

.386
.model flat, stdcall

option casemap:none

include \masm32\include\windows.inc
include \masm32\include\kernel32.inc
include \masm32\include\user32.inc
includelib \masm32\lib\kernel32.lib
includelib \masm32\lib\user32.lib
;=================================================================


; prototype of my function

GetFileName PROTO :DWORD, :DWORD


;=================================================================

.data

  ThePath db "C:\ForTest\AnyFile.exe",0

;=================================================================

.data?

 
  TheFile db 50 dup(?)


;=================================================================

 

.code




comment *

my idea is:
1- Get the length of the full path
2- from the end of the path, Go back until we find "\"
3- Add 1 to that position, so we get the first char after "\"
4- copy all the chars from that position to the end of the path, to the destination "Buffer"
*




; Begin in the real work :-)


GetFileName proc lpPath:DWORD, dstBuffer:DWORD

mov eax, lpPath ; get the address of lpPath
xor edx, edx    ; edx = 0

@@:
mov cl, BYTE PTR [eax]
add eax, 1
inc edx
cmp cl, 0       ;used this, because i think there is a "zero" after the string
jne @B

; all code under previous @@, is for getting the length of the path, and put it in "edx"


mov eax, lpPath
add edx, eax
cmp BYTE PTR [edx], "\"
jne @F

@@:
dec edx
cmp BYTE PTR [edx], "\"
jne @B

; all code after the previous comment, if for getting the position of "\" , and put it in "edx"



inc edx         ; get the position of first char of the Real File Name :-)

mov eax, lpPath

add eax, edx    ; eax now has the address of the first char after "\" char

xor edx, edx     ; edx = 0, for using it in counting

push esi    ; wrote this because, as i understood, i have to preserve this reg as well as (ebx, edi), correct me
            ; if i'm wrong here, or if there are other registers must preserved ...


mov esi, dstBuffer   ; put the address of dstBuffer in "esi"

@@:
mov cl, BYTE PTR [eax + edx]
mov BYTE PTR [esi + edx], cl
inc edx
cmp cl, 0  ; used this, because i think there is a "zero" after the string
jne @B

pop esi         ; get back the value which was in esi

mov eax, edx    ; the return value is the length of the FileName  :)

ret           

GetFileName endp




  ; here is the test of GetFileName
 
  start:
  invoke GetFileName, Addr ThePath, Addr TheFile
  invoke MessageBox, 0, Addr TheFile, Addr TheFile, MB_OK
  invoke ExitProcess,0
  end start
 

; last comment: the program is crashed immediatily after running it !!!




thank you alot in advance :-)

[Ossa]

[edit] Realised this might sound a bit harsh/terse - it's not meant to be - I'm just in a rush [/edit]

Your main problem is here:

Code Select Expand

; all code after the previous comment, if for getting the position of "\" , and put it in "edx"

edx actually holds the address of the "\", not the offset into the string. This means that on these lines you problem continues:

Code Select Expand

add eax, edx    ; eax now has the address of the first char after "\" char

as eax now points to rubbish. So when you do:

Code Select Expand

mov cl, BYTE PTR [eax + edx]

You will be accessing invalid memory locations and your program will crash.

There are many thing you can do to fix this, but the easiest idea is to replace:

Code Select Expand

mov eax, lpPath

add eax, edx    ; eax now has the address of the first char after "\" char

with:

Code Select Expand

mov eax, edx



The other mistake is:

Code Select Expand

mov eax, lpPath
add edx, eax
cmp BYTE PTR [edx], "\"
jne @F

@@:
dec edx

The cmp/jne does nothing useful.

Ossa

[edit] If you read through your code, you will see that there is a way to do this using only pointers without using offsets into strings. [/edit]

[P1]

From the MASM32 library:
NameFromPath proc lpPath:DWORD,lpBuffer:DWORD

Description

NameFromPath reads the filename from a complete path and returns it in the buffer specified in the parameter list.

Parameters
   1. lpPath is the address of the full path that has the file name.

   2. lpBuffer is the address of the buffer to receive the filename.

Return Value
The file name is returned in the buffer supplied in the second parameter.

Comments
The buffer to receive the filename must be large enough to acept the filename. For safety reasons if dealing with both long path and file name, the buffer can be made the same length as the source buffer.

Also use "Search", we had a good discussion on this, with many techniques discussed.

Regards,  P1  :8)

[Shantanu Gadgil]

Hi Dasar,
the comments I have posted, though might seem terse, they were just quick "jot-downs" as I was reading through your code side-by-side. Each with its own heading  :bg :bg

praises:
well worked out algo...thats the way to think  :U

comment:
when target is byte no need to write "byte ptr"

suggestions/bugs(?):
save restore regs at top and end
use MAX_PATH for buf lengths
double line spacing not reqiured (for post text) :bdg
code indentation ???  :red :eek

debatable (lets just "my way of thinking"  :lol ):
be familiar with the so called "high level constructs"  :lol They are your friends  :green
.while/.endw, .if/.endif, etc  :8)

Code Select Expand


GetFileNameX proc pszFullFileName:DWORD, pszOutFileName:DWORD
push esi
push edi

mov esi,pszFullFileName
.while byte ptr [esi] != 0 ;go till end of fullfilename
inc esi
.endw

.while byte ptr [esi] != '\' ;move back till last '\'
dec esi
.endw
inc esi ; point to the next char after the '\'

xor eax,eax ;counter
mov edi, pszOutFileName
.while byte ptr [esi] != 0
movsb
inc eax
.endw
mov byte ptr [edi], 0

pop edi
pop esi
ret
GetFileNameX endp

[Dasar]

thank you Ossa for your help, i'll try what you said

@P1 thank you for the code

but i said:

i know that maybe there is a function in masm32 does the same mission, but i didn't search for any function, because the main

purpose of writing this function, is the training on write functions in asm...

 

:)

and i'll search for the topic you mention above :)


shantanu_gadgil thank you for your useful comments and your code :)

be familiar with the so called "high level constructs"   They are your friends 
.while/.endw, .if/.endif, etc

yes, i know these instructions, but i want to be familiar with "push, pop, mov, add, xor, call, cmp, jmp, jn, jne, etc..."   :)




any other hints or comments about my code from anyone is very appreciated  ^_^

[Ratch]

Dasar,

please also tell me if there is an easier and better way to write such funcion.

     Well, yes, maybe.  There is a book called Programming Windows, 5th edition,  by Charles Petzold.  In the 11th chapter he submits a C program called POPPAD3, which uses the Common Dialog Boxes.  This program opens files using filters like *.*,*.txt,*.bak, etc.  He does it by using the Common Dialog API like GetOpenFileName, of which you can read the documentation.  The program opens files, searches for text, reads text replaces text.  In other words, it is a poor man's file editor written to showcase the Common Dialog Boxes.  This might be too heavy for what you want, but it does give an alternative high level way of implementing what you are doing.  I have translated that program from C to MASM if you are interested.  If not, I just thought I would mention the Common Dialog Boxes for you to peruse.  Ratch

[white scorpion]

Funny...
I wrote exactly the same function for a program of mine. it even has the same name  :bg

Code Select Expand


GetFileName PROC FileNameLocal:DWORD,FileNameLocalSize:DWORD
    invoke GetModuleFileName,NULL,FileNameLocal,FileNameLocalSize
    invoke lstrlen,FileNameLocal
    mov ecx,eax
    mov eax,FileNameLocal
    add eax,ecx
@@:
    cmp byte ptr [eax],5Ch   ;first '\' from the back
    jz @F
    dec eax
    cmp eax,FileNameLocal
    jnz @B
    xor eax,eax
    ret
@@:
    inc eax
    invoke lstrcpy,FileNameLocal,eax
    xor eax,eax
    ret
GetFileName ENDP
;***********************************************************

Since i needed the name of only the program currently running, i put in GetModuleFileName to supply the data, but you could do it without that function as well.

here's a function changed to take as argument the complete path and which will use the same buffer to put the result in:

Code Select Expand


GetFileName PROC FileNameLocal:DWORD
    invoke lstrlen,FileNameLocal
    test eax,eax
    jnz @F
    xor eax,eax
    dec eax
    ret
@@:
    add eax,FileNameLocal
@@:
    cmp byte ptr [eax],5Ch   ;first '\' from the back
    jz @F
    dec eax
    cmp eax,FileNameLocal
    jnz @B
    xor eax,eax
    ret
@@:
    inc eax
    invoke lstrcpy,FileNameLocal,eax
    xor eax,eax
    ret
GetFileName ENDP


The latter one should perfectly fit your needs, total size = 55 bytes

[Casper]

White Scorpion,

There are a couple of things worth mentioning...

Code Select Expand

@@:
    cmp byte ptr [eax],5Ch   ;first '\' from the back
    jz @F
    cmp eax,FileNameLocal
    jnz @B
    xor eax,eax
    ret
@@:
The above code will test the same byte forever.  You are missing the code to decrement eax.  This is probably just an oversight but I thought I better mention it because there are those who just copy and paste code that we post.

In the both postings, however, when you abort because you have reached the beginning of the string you are setting the value of eax to zero when you should set it to one to report the error.

hth,
Casper

[white scorpion]

huh where is the dec eax missing ? :bg
Updated above code, must have accidentally removed it when updating the code ;)

2nd thing: the reason i end with 0 is that there actually is no real error:
- the stringsize is bigger then 0
- there is no \ in the string
- this means that the inputted string is already only the filename.
and since it returns the filename in the original buffer, the resulting filename can still be used as only a filename.

i can change it without any problems, and so can anyone else, but i decided it would be best to do it like this.

[Mark Jones]

Dasar, have you tried stepping through your code in a debugger?

Load it using OllyDbg and keep pressing F8. This will let you see what happens as each line of code is processed. With some practice it will become trivial to fix bugs like what you are experiencing. :U

[dsouza123]

Dasar

  Ossa nailed it, your program has one show stopping bug that kept your proc from working.
edx has an address not an index into the string.
Fortunately with 1 more instruction the procedure, thus program, works as intended.

Code Select Expand


    inc edx         ; get the position of first char of the Real File Name :-)

    mov eax, lpPath

                    ; this is the critical missing instruction, with this your program works !
    sub edx, eax    ; because edx is an address, to turn it into an index, subtract the address of the start of the string



dsouza123

[dsouza123]

Dasar

  Since you are already are scanning the entire string for the terminating 0,
the test for a \ can be integrated at no cost, and omit later rescanning the string for the \.
  A convention: esi has the address of a source string, edi has the address of a destination string.

dsouza123

Code Select Expand


GetFileName proc lpPath:DWORD, dstBuffer:DWORD
    push esi
    push edi

    mov esi, lpPath ; get the address of lpPath
    mov eax, esi    ; eax has address of lpPath, and will have a valid address from the string just in case

    @@:
    mov cl, BYTE PTR [esi]
    add esi, 1
    cmp cl, "\"
    jne pastbackslashtest
    mov eax, esi    ; eax now has the address of the first char after "\" char
pastbackslashtest:
    cmp cl, 0       ; used this, because i think there is a "zero" after the string
    jne @B

    xor edx, edx    ; edx = 0, for using it in counting

    mov edi, dstBuffer   ; put the address of dstBuffer in "edi"

    @@:
    mov cl, BYTE PTR [eax + edx]
    mov BYTE PTR [edi + edx], cl
    inc edx
    cmp cl, 0       ; used this, because i think there is a "zero" after the string
    jne @B

    pop edi
    pop esi         ; get back the value which was in esi

    mov eax, edx    ; the return value is the length of the FileName  :)

    ret
GetFileName endp


[Dasar]

@ Ratch

I have translated that program from C to MASM if you are interested

yes, if you can, please give me the code, i might need it oneday soon :)



@White Scorpion

Funny...
I wrote exactly the same function for a program of mine. it even has the same name

lol, yes that was funny :D  , and thank you alot for your code :)


@ Casper

This is probably just an oversight but I thought I better mention it because there are those who just copy and paste code that we post.

thank you for the note, btw i'm one of those whom do not use copy/paste  :D



@Mark Jones

no, i didn't use a debugger, because i nearly don't know anything about debugging, but it seems that i will have an experience with them soon :)

thank you for your reply



@dsouza123

thank you alot for your help and comments  :-)

[Jimg]

Another thing you might consider is just saving the name the first time through the string-

Code Select Expand


GetFileName proc uses esi edi lpPath:DWORD, dstBuffer:DWORD

       mov esi,lpPath     ; where to get the input
reset: mov edi,dstBuffer  ; where to store the output
@@:    lodsb              ; get a character
       cmp al,'\'         ; is it start of possible file name?
       je reset           ; yes, discard this character and reset output address
       stosb              ; no, save possible file name character
       or al,al           ; are we done?
       jne @b             ; no, get another character

       mov eax,edi        ; compute length of output
       sub eax,dstBuffer
       dec eax
       ret

[Dasar]

thank you Jimg :)