string comparison in ASM

Question

I'm new to assembler and I'm trying to do a simple string comparison. The thing is it always displays the message saying the strings are identical, even if they are not. Here's the code:

.model small
.data 
msg_incetermin db " ****************************$"
msg1 db " First string: $"
msg2 db " Second string: $"
msg3 db " str1 is ->$"
msgne db " strings are equal$"
msge db " strings are not equal$"
msg_banoutput db " -----------------------------$"
msg_termin db " end of program ...$"
sir1 db 80 dup(?),'$'
sir2 db 80 dup(?), '$'
newl db 13, 10,'$'

.code

; definitions -----------------
afis macro %text ; dispay string macro
lea dx, %text[1]
mov ah, 09h
int 21h
endm

; read string macro using linefeed 0ah
citire macro %str
mov %str[0], 75
lea dx, %str
mov ah, 0ah
int 21h
mov bl, %str[1]
mov %str[1], ''
add bl, 2
mov %str[bx], '$'
endm
;------------- definitions

mov ax, dgroup
mov ds, ax ;reserving source and destination memory segments
mov es, ax

afis newl ;intro messages
afis msg_incetermin
afis newl
afis msg1
citire sir1
afis newl
afis msg3
afis sir1
afis newl
afis newl
afis msg2
citire sir2
afis newl
afis msg3
afis sir2
afis newl
afis newl
afis msg_banoutput

lea si, sir1
lea di, sir2

cld
mov cx, 100
rep cmpsb
jne mes0

afis newl
afis msgne

jmp over1
mes0:
afis newl
afis msge
over1:


peste2:
afis newl ;exit messages
afis msg_incetermin
afis newl
afis newl
afis msg_termin
afis newl
mov ah, 4ch
int 21h
end  

Any suggestions in how I can code better in ASM, or in how I can make this program work or work better will be welcomed.

Thanks for your time.

Answer 1

A couple of problems:

1. You've got a hardcoded comparison count, so even if the strings are equal you'll compare data beyond the end of the strings unless they're both 100 bytes long (which they won't be, since you've declared them to hold 81 bytes each).

mov cx, 100

You should compare at most length_of_the_shortest_string bytes. If the strings aren't the same length they're not equal and there's no need to compare their contents at all.

---

2.

rep cmpsb

Not necessarily a problem, since the assembler probably takes care of this for you, but the correct prefix to use here would be repe.

---

I also noticed a lot of poorly named labels. For example, you've named the "strings are equal" string msgne which implies that it's a message that should be shown when the strings are not equal. Then there are labels like mes0 that says nothing about their purpose. Things like that makes code unnecessarily hard to read/debug.