Assembly x86 - counting small letters in a file

Question

I am working on an assignment and I got stuck somehow. This is the part of the code, that must be wrong. And what is actually wrong is that when the program should print the number (thtough procedure convertNumber, which works fine!), it doesn't do anything, neither it returns. It just doesn't react, I must close it manually.

read:
            mov ah, 3fh                 ; function for reading from file
            mov bx, handle              ; moving file handle to BX
            mov cx, 10000               ; how many bytes will be read
            mov dx, buffer              ; where the string will be saved
            int 21h                     ; system call ready

            mov bytesRead, 0            ; nulling my variable bytesRead, which represents how many bytes have been read
            mov bytesRead, ax           ; now moving the real value from AX

            cmp ax, 0                   ; if it is equal to 0, then there is nothing left to read                                                     
            je closeFile                ; close file

            mov cx, 0                   ; starting position

cycle:
            cmp cx, bytesRead           ; if I need to reload buffer
            jge read

            mov bx, 0                   ; nulling position counter
            mov bx, cx                  ; moving CX to BX -> mov ax, buffer[cx] was an illegal operation, so I did it this way
            mov ax, buffer[bx]          ; getting a character from BX. position from buffer

            cmp ax, 'a'
            jb next                     ; below 'a', skip
            cmp ax, 'z'
            ja next                     ; above 'z', skip
            inc count_task              ; if it passed, it is between 'a' and 'z'
            jmp next                    ; and moving on

next:
            inc cx                      ; increase position counter (always!)
            jmp cycle                   ; and repeat

finally:
            mov ax, count_task          ; move the final count to AX (needed by the procedure convertNumber)
            call convertNumber          ; convert number to its ASCII value (+48)
            ret

error_fopen:
            print fopenErr
            ret

error_fclose:
            print fcloseErr
            ret

closeFile:
            mov ah, 3eh
            mov bx, handle
            int 21h

            jc error_fclose
            jmp finally

convertNumber proc near
    push si

    mov buff, '$'
    lea si, buff
    mov cx, 10

conv:
    mov dx, 0
    div cx                          ; AX is being divided by CX, where value 10 is stored (direct dividing by 10 is not legal)
    add dx, 48                      ; getting ASCII value of the number (from 0 to '0')
    dec si                          ; going backwards
    mov [si], dl
    cmp ax, 0
    jz printNumber                  ; if AX is empty (0), print the number
    jmp conv                        ; else loop 

printNumber:
    mov ah, 9                       ; service to print string
    mov dx, si                      ; position of my string (number)
    int 21h

    pop si

    ret
convertNumber endp

I hope you can understand what is going on in this code, if not, I can explain you. I would be very grateful for any help!

You have good label names, but you're not commenting each line. If you will provide a short descriptive comment on each line of your source code, you will receive a much more enthusiastic response, with much greater levels of help. — User.1, Apr 04 '15 at 20:15
Why do you `mov bx,0` then do `mov bx,cx` before `mov ax, buffer[bx]`? By the way, this would be an ideal application for the `loop` and `lodsb` instructions. You should check them out. :) As @User.1 suggests, for good reasons you should comment your instructions. Sometimes in the process of doing that, you'll discover the problem. — lurker, Apr 04 '15 at 20:18
Use a debugger to single step the code and see where it goes wrong. — Jester, Apr 04 '15 at 20:39
@User.1 yea, sorry, my bad, I commented almost every line now — pitbbul, Apr 04 '15 at 20:48
@lurker I wanted to do it that way, but tasm said it is illegal or something like that, so I changed it that way, should be the same, but it is using more registers... well, I even should apply some of the string operations, but I didn't know how so I did it this way... — pitbbul, Apr 04 '15 at 20:54
@Jester well, if I only knew how to debug in Turbo Debugger :( my friend tried to debug it and he said it has a problem with CALL (it is called from another file, but it worked when I was just trying some simple echos) — pitbbul, Apr 04 '15 at 20:55
TASM should know those instructions, unless you use incorrect syntax. What about the problem I pointed out? — lurker, Apr 04 '15 at 20:57
This pops out at me, double check it, `mov ax, buffer[bx] ; getting a character from BX. position from buffer` Make sure that you want 16 bits instead of 8. Just something that showed up, I haven't really examined why you are doing 16 bits there. I'll keep reading. — User.1, Apr 04 '15 at 21:03
I think that's it. Check your assembly source listings after assembling. I think you'll see that you are comparing `'a'` and `'z'` which are `61h` and `7Ah` respectively, against who-knows-what in the 16 bits of the `Ax` register. Put a breakpoint right after you load `Ax` and I'll bet you see a 16 bit number; I'm guessing that it's almost always above `7Ah` and is causing your intended logic to never happen. — User.1, Apr 04 '15 at 21:09
ooh yeah guys, thanks @User1.3 , I fixed it the problem was really in mov ax, buffer[bx] , I changed it to mov al, byte ptr buffer[bx] and now it works :) — pitbbul, Apr 04 '15 at 21:09
The `byte ptr` designation is probably optional since the assembler knows you're moving to `al`, but that's assembler dependent. — lurker, Apr 04 '15 at 21:25
@lurker well, without it it was illegal, so I had to do it that way :) by the way, now I am working on another task - print the positions of those small letters :) any hints? — pitbbul, Apr 04 '15 at 21:34
OK, probably assembler dependent. You don't need to load a `0` into a location before you load a value if the size is the same. So `mov ax, 0` followed by `mov ax,something` can just be `mov ax, something`. You should ask a new question for your new task. How do you want the positions printed? Just as a number per line on the output? — lurker, Apr 04 '15 at 21:37
oh, you are right, it will overwrite the whole register... it is just a mini-task, if I have some big issues with that, I will create a new question... I would like to have them in one line(or more), separated by commas (,) — pitbbul, Apr 04 '15 at 21:42
All you need to do is keep an index count starting at 0 for your loop, increment on each time through the loop, and call your number print routine on it each time you find a small letter. — lurker, Apr 04 '15 at 21:48
Where is `buff` define? You are loading the start address of `buff` into `si`, and then *decrementing* `si` and storing ASCII digits there. So it appears you're storing them someplace in memory *before* the `buff` label. — lurker, Apr 08 '15 at 11:17
@lurker it is defined in main data segment as `buff db 10 dup(?)` I don't understand it very well, I just edited friend's procedure a bit — pitbbul, Apr 08 '15 at 11:37
oh, now I get it... he defined it as first variable in data segment, maybe I should do that too... or is there some better solution? — pitbbul, Apr 08 '15 at 11:47

lurker · Accepted Answer · 2015-04-06T00:49:12.157

0

A version using lodsb and loop shown below, with a couple of other things tidied up. I added a little extra to print the index of each small letter found. I didn't check this for silly errors as I'm not set up to run DOS interrupts.

This code will handle files longer than 65535 bytes long, but the "convertNumber" subroutine would need to be updated to print long integers.

EDIT: The format below assumes the NASM assembler (the actual assembler used isn't mentioned in the problem statemet).

        mov     [count_task], 0
        mov     [count_task+2], 0
        mov     [index], 0
        mov     [index+2], 0
read:
        mov     ah, 3fh                 ; function for reading from file
        mov     bx, [handle]            ; moving file handle to BX
        mov     cx, 10000               ; how many bytes will be read
        mov     dx, buffer              ; where the string will be saved
        int     21h                     ; system call ready
        jc      error_read

        cmp     ax, 0                   ; if it is equal to 0, then there is nothing left to read
        je      finally                 ; print result and close

        ; This code assumes your ds segment register already points to your data

        lea     si, [buffer]
        mov     cx, ax                  ; starting loop counter
        cld                             ; clear direction flag (increment)
cycle:
        lodsb
        cmp     al, 'a'
        jb      next
        cmp     al, 'z'
        ja      next
        add     [count_task], 1
        adc     [count_task+2], 0

        ; Print the current index of the found small letter.
        ; Note it's assumed that convertNumber saves certain registers
        ; according to calling conventions. For example, if it uses "si"
        ; then it should save "si". AX, CX, and DX are fair game.
        ;
        push    cx
        mov     ax, [index]
        mov     dx, [index+2]
        call    convertNumber           ; print the index of the found small letter
        pop     cx
next:
        add     [index], 1              ; increment overall index
        adc     [index+2], 0
        loop    cycle                   ; and repeat

        jmp     read                    ; read another buffer full
finally:
        mov     ah, 3eh
        mov     bx, [handle]
        int     21h
        jc      error_fclose

        mov     ax, [count_task]        ; move the final count to DX:AX (needed by the procedure convertNumber)
        mov     dx, [count_task+2]
        call    convertNumber           ; convert number to its ASCII value (+48)
        ret

error_read:
        print   freadErr
        ret

error_fopen:
        print   fopenErr
        ret

error_fclose:
        print   fcloseErr
        ret

edited Apr 06 '15 at 00:49

answered Apr 04 '15 at 21:47

lurker

56,987
9
69
103

oh, thank you very much! by the way, that nulling of count_task in label read -> do you think it will work well? when I reload the buffer the number of letters will be replaced by 0, won't it? I was thinking about the second task (positions of those letters), I wanted to print the number in CX in fact (followed by a comma), but it got into endless loop writing 1,1,1,1 or something like that, it was moving really fast :D to complete this up, how would you do that in the code above? thanks – pitbbul Apr 04 '15 at 22:10
@pitbul, Oops, my bad. `count_task` should be set to 0 before `read`. So should `index`. Why do you want to write what's in `CX`? That's just a register. Exactly what do you want to write is the real question. `CX` should just hold a value that helps your program do a loop. My loop is structured differently than yours, since `CX` counts down with a `loop` instruction. – lurker Apr 04 '15 at 22:13
it happens :) I kept the actual position in string in CX, so I want to print that... I've noticed, that you did it other way (also better :) ) – pitbbul Apr 04 '15 at 22:16
@pitbul Yeah I wanted to use `loop` which means `cx` had to be the loop counter out of necessity. – lurker Apr 04 '15 at 22:17
yeah, I know how it works :) also, my program should be able to open files more then 128 kilobytes, how would you do that? and how to define the index variable? – pitbbul Apr 04 '15 at 22:30
@pitbbul you'll need to define `count_task` and `index` as `dw` memory entries (double word). When you increment them, you'll add 1 to the low word first, then do an *add with carry* to the high word. You'll also need to update your number printing function to handle a 32-bit value. I updated my answer with the long `index` and `count_task` variables (I might not have the correct syntax per your assembler). – lurker Apr 04 '15 at 22:41
yes, I have defined them as DW variables... meanwhile I fixed that convertNumber procedure to save SI, AX, CX and DX and now it works :) just need to separate them with commas – pitbbul Apr 04 '15 at 22:45
@pitbul Technically, it doesn't have to save `AX`, `CX`, or `DX` since it's assumed by standard convention that those can be "clobbered" and should be saved by the caller only if needed. `SI` must be saved, though. See, for example, [x86 Assembly Guide](http://www.cs.virginia.edu/~evans/cs216/guides/x86.html), under *Caller Rules*, etc. – lurker Apr 04 '15 at 22:46
@pitbbul glad I could help! I think even though you're new on SO, you can check the "accept" check mark on my answer (I get points, but no prizes ;)). – lurker Apr 04 '15 at 23:00
@pitbbul I forgot to mention: beware of `inc` when doing 32-bit increments. `inc` does not affect the carry flag, which is why I do `add variable, 1` instead of `inc variable` when it is part of a double word. – lurker Apr 04 '15 at 23:07
hm, thanks for noticing, but it does not work correctly as you have written, it is the same issue in fact, it somehow freezes and does not react... in another sub-task in my assignment, I am counting the size of the file, and I use 2 variables, one to store last 3 digits (max 999) and one to store other 3 digits (altogether it can be 999 999 bytes, which is enough) – pitbbul Apr 05 '15 at 18:16
@pitbul sorry no warranty expressed or implied. I mentioned that I don't have a platform setup to run the DOS interrupts. It's probably a small error somewhere, not a conceptual one. The method is right I think. And you have to make sure your `convertNumber` for long values works correctly. Since this is an assignment, in think it's a reasonable exercise left to the reader to find the bug. – lurker Apr 05 '15 at 18:21
@pitbbul I did find an omission where i forgot to load `dx` with the final high word of the count. Also, you haven't said what assembler you're using, so the syntax can vary depending. I updated the answer assuming NASM syntax. In general, any assembler distinguishes referencing a label by its *value* (address) (*e.g.*, `count` in NASM) versus it's *contents* (*e.g.*, `[count]` in NASM). – lurker Apr 06 '15 at 00:51
your update did not change the thing... maybe there is a problem with convertNumber procedure, I will post it below this comment – pitbbul Apr 07 '15 at 11:11
@pitbul OK, I don't have TASM. But you should be able to translate what I have in NASM over to TASM. Please put `convertNumber` into your original problem statement for clarity. – lurker Apr 07 '15 at 11:12
I updated the original post, so please check it there, thanks – pitbbul Apr 08 '15 at 10:47

Assembly x86 - counting small letters in a file

1 Answers1