If you execute this code with
n DB 0, it returns without setting
ch to any value, leaving the result as undefined.
n happens to be between 128 and 255, the
jge will treat the comparison as if it was a signed comparison, and since 0 is not greater than or equal to any value between -128 and -1, inclusive, immediately jump to the end, again leaving
ch as an undefined value.
You should use
jae instead, so you can have up to 255 values in
Does your array of values represent unsigned bytes, or signed bytes? If your array contains
0FFh, is that the largest possible value, or one smaller than zero?
If the data is supposed to be signed, you're good. If it was supposed to be unsigned bytes, you again should change the
Unnecessary Register Usage
mov cx,0h, but nowhere do you use
cl, so are unnecessarily destroying
mov ah,[bx+si] and use
mov ax,0h to clean-up at the end. Again, nowhere have you used
al, so you've unnecessarily destroyed
If you used
cl instead of
ah, you wouldn't need to destroy
ax at all.
si appears unnecessary. Instead of based-index addressing mode:
you could use register indirect addressing mode:
cx is the standard for loop counts.
ax is the standard for function return values.
si is the standard for source indexing. So ...
Have your function store and return the maximum in
ah to fetch values from the
array, and zero it at the end. As a bonus,
ax could also be considered the return value. (If the array is intended to hold signed data, then use
cbw to sign extend
ax instead of zeroing
If you used
cx for your array length, and decrement to zero, you could use one
LOOP instruction instead of a pair of instructions to perform the compare-and-branch. As another bonus, it will already be zero by the end, so you won't need to clear it.
array address in
si, and use
cx for double duty by using based-index addressing (
mov ah, [cx+si]) to index into your
array. Of course, you'll be searching backwards, which still fine for finding the maximum. You'd just have to be careful to include the 0th value, and exclude the nth value (
dec cx before the fetch, not afterwards).
- return value in
cx zeroed (automatically)
si cleared (manually)
- All other registers unchanged.