Flaw in decimal to binary code

[Magnum]

This code works well, but I just found out that it freezes if you input a zero.

How can I test for zero?

Thanks.

;dec2bin.ASM - Hugh Noland
;
;Prints out binary representation of decimal input from keyboard,
;which cannot exceed 4,294,967,295.

cseg   segment
assume cs:cseg, ds:cseg
org 100h
.386

begin: jmp main

msg       db     10,13,"Enter a decimal number: $"
crlf      db     10,13,"$"
buf       db     13,?,11 dup(?)
ten       dd     10

main:
   mov dx,offset msg             ;call for input
   mov ah,9                      ;
   int 21h                       ;

   mov dx,offset buf
   call read_str                 ;Dos read-string function.  Input will
                                 ;be stored as string in buf
   mov dx,offset crlf            ;
   int 21h                       ;print carriage return, line feed

   mov si,offset buf
   inc si
   mov cl,[si]                   ;get string length in cx
   mov ch,0                      ;
   jcxz exit                     ;exit if no input
   dec cx                        ;dec cx so as not to multiply last
                                 ;digit by 10
   inc si                        ;ds:si==>first digit
   xor edx,edx
   xor eax,eax
   jcxz dig2
dig1:
   xor ebx,ebx
   mov bl,[si]                   ;get digit
   inc si
   sub bl,30h                    ;convert ASCII to decimal
   add eax,ebx                   ;add to previous result
   mul ten                       ;multiply sum by 10
   loop dig1
dig2:
   xor ebx,ebx
   mov bl,[si]                   ;get last digit
   sub bl,30h
   add eax,ebx                   ;add to previous result
   mov ebx,eax
   call convert                  ;call routine convert to binary and

print

exit:
   mov ax,4c00h
   int 21h


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

---
;Reads string input from keyboard and echoes character to screen.
;On entry ds:dx points to storage buffer.  User stores size of buffer
;in first byte of buffer.  The string length is returned in second
;byte of buffer.  String storage begins at third byte.
;Backspace may be used for editing during input.  Input is terminated
;by pressing enter, which is read into the string.  Therefore effective
;length of string = length indicated in first byte minus 1.  Length of

buffer
;should equal length of string + 3 (first 2 bytes must be accomodated).
;------------------------------------------------------------------------

---
read_str   proc
   push ax
   mov ah,0ah
   int 21h
   pop ax
   ret
read_str   endp

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

---
;Prints number in EBX in binary
;------------------------------------------------------------------------

---
convert   proc
   mov cx,33                     ;CX will count # of binary digits
b1:                              ;after discarding leading
   dec cl                        ;binary 0's
b2:                              ;
   rcl ebx,1                     ;
   jnc b1                        ;

   pushf                         ;preserve flags
   mov ax,cx                     ;
   push cx                       ;get remainder when # of digits is
   mov cx,4                      ;divided by 4
   xor dx,dx                     ;
   div cx                        ;
   pop cx                        ;
   mov si,dx                     ;and store in SI
   popf                          ;restore flags

   mov ah,2                      ;print using Dos function 2
b3:
   mov dl,0
   adc dl,30h
   cmp si,0                      ;when SI = 0, print a space
   jne b4                        ;
   push dx                       ;save DX
   mov dl,' '                    ;
   int 21h                       ;
   pop dx                        ;restore DX
   mov si,4                      ;after first group of digits
                                 ;print in groups of 4
b4:
   int 21h
   dec si
   rcl ebx,1
   loop b3
   ret
convert   endp

cseg   ends
end   begin

[dedndave]

one thing...
the "buffer size" value should not count the sizes
and, because you cannot represent more than 4,294,967,296, the buffer only need be 10 bytes, not counting the size bytes

buf       db     10,?,10 dup(?)

also, i think if you are going to use 32-bit registers, you need to zero the high dword of ecx for LOOP to work correctly
i wouldn't use LOOP
i would use dec cx - you avoid the ecx/cx problem and loop
another little problem you may have is the mul instruction clobbers the edx register
but, you can write it so that it only multiplies for all but the last digit

xor eax,eax
mov edi,eax
mov ebx,10

loop00:
mov al,[si]
and eax,0Fh
inc si
add eax,edi
dec cx
jz loop_done

mul ebx
mov edi,eax
jmp loop00

loop_done:
;result in eax

there are more efficient ways to write it, but you get the idea

[MichaelW]

IIRC in 16-bit code LOOP will always use CX. To force the use of ECX you must specify LOOPD.

[dedndave]

ah - ty Michael
after writing so much 32-bit stuff, i forget 16-bit
it actually looks a little foriegn to me to use "[si]" - lol
i suppose lodsb would work just as well - no inc si, then
not like speed is super critical

one comment i might make is - use the single charcter input function
that way, you can filter out non-numeric characters, and put it inside the mul loop
then - if they try to put in a number bigger than 4,294,967,295, you can terminate input

[FORTRANS]

Hi,

   Dave, the input buffer also needs to contain the carriage return.
That's why it's 11 for a ten digit number.

   After a quick look, there is no check for zero, and there is the
discard of leading zero bits affecting the count in CX.  Test EBX
for zero before calling "convert".

Regards,

Steve N.

[dedndave]

ah yes - lol - forgot about that
still, the count should not include the count bytes
when i wrote 16-bit code, i rarely used the dos interrupts for keyboard input
i used bios interrupt 16 functions 0 and 1, and echoed characters using INT 29h so they were not re-directed
that way, you write your own code to filter characters and limit input value and length

[FORTRANS]

Hi,

   Should have said that the JNC will cause an infinite loop.
Always see things after posting...

   Yes, BIOS input allows more control, and DOS fn 0AH
allows more convenience.  I even tried to get input using
the DOSKEY functions to allow for more editting.  And
combining that with keyboard stuffing for default input
values.  C'est la vie?

Cheers,

Steve

[dedndave]

yup - that jnc is why zero doesn't work - lol

[Magnum]

   After a quick look, there is no check for zero, and there is the
discard of leading zero bits affecting the count in CX.  Test EBX
for zero before calling "convert".

Regards,

Steve N.

Thanks, I put this in and it works OK.
Is there still a problem with the jnc statement.

Andy


cmp ebx,0              ; check for zero, impt. or else Windows FREEZES if a zero is input!!
je exit

[FORTRANS]

Hi Andy,

Is there still a problem with the jnc statement.

   Not if the program is good enough for you.  The idea was that the
"convert" routine was trying to find the number of set bits in EBX.
To that end, it put a count in CX, and shifted the bits out of EBX
until a set bit was found.  The bits going out in a shift set the carry
flag if a one.  Or reset the carry flag if a zero.  If EBX is zero, it has
no one bits, so carry never gets set, and presto, infinite loop.

   You could fix that by testing CX and exit the loop when it gets to
one (or zero?).  Or by rewriting the loop to use the LOOP instruction
to limit the number of times the loop is executed.

HTH,

Steve