Link to home
Start Free TrialLog in
Avatar of Sandra-24
Sandra-24

asked on

cmp [esi], [edi] ?

The compiler doesn't like the line: cmp [edi], [esi], "bad operand type"

Why?

Also would you have a look at what I'm doing with the compare loop (and the reason I am trying to do the above cmp) and see if you can think of a better way of doing it. Keep in mind this hasn't been run yet so there may be bugs (my first assembly program of > 3 lines :).

void * __cdecl memmem(const void * buf, size_type count, const void * needle, size_type needle_len)
{
      __asm
      {
            pop            ebx                                    ;// Put needle length in ebx register
            pop            edi                                    ;// Put buffer in destination register
            pop            ecx                                    ;// Put count in ecx register
            pop            esi                                    ;// Put needle in source register

            test      ebx, ebx                        ;// Is needle empty?
            jz            found                              ;// Return buffer

            xor            edx, edx                        ;//      Clear edx, because we may use the whole thing later (in to_memchr)

            mov            dl, [edi]                        ;// Put first byte of needle into dl (we know it has atleast 1 byte

            cmp            ebx, 2                              ;// Is needle atleast 2 bytes?
            jb            to_memchr                        ;// Needle length is one byte, call memchr instead

            mov            dh, [edi+1]                        ;// Put second byte in dh
            sub            ebx, 2                              ;// Subtract two from needle length (since we have two bytes of it in registers already)

            jmp            main_loop                        ;// Needle is greater than or equal to 2 bytes

cmp_next_byte:
            cmp            [edi], dl                        ;// Compare first byte of needle with byte of buffer
            je            get_second_byte                  ;// Match, check the rest of needle against the rest of buffer

            jmp            main_loop                        ;// Back to main loop

get_second_byte:
            sub            ecx, 1                              ;// Decrement counter
            jb            not_found                        ;// Counter went below zero, needle wasn't found

            add            edi, 1                              ;// Increment buffer pointer
            cmp            [edi], dh                        ;// Check if second byte matches
            je            compare_init                  ;// Match, compare rest of needle against buffer

            jmp            cmp_next_byte                  ;// We've fetched the next byte, decremented the counter, just continue on as normal, comparing it with the start of the pattern

compare_init:
            cmp            ecx,ebx                              ;// ebx must be greater than or equal to length of needle - 2
            jb            not_found                        ;// If count is < needle length, needle cannot be in the buffer

            mov            eax, ebx                        ;// Take a new copy of needle length for use as a counter

            push      edi                                    ;// Preserve buffer pointer
            push      esi                                    ;// Preserve needle

            jmp            compare_loop                  ;// Loop

compare_cleanup:
            pop            esi                                    ;// Restore needle
            pop            edi                                    ;// Restore buffer

            jmp            main_loop                        ;// Continue looking

main_loop:
            sub            ecx, 1                              ;// Decrement counter
            jb            not_found                        ;// Counter went below zero, needle wasn't found

            add            edi, 1                              ;// Increment buffer pointer

            jmp            cmp_next_byte                  ;// Compare this byte of the buffer against dl

compare_loop:
            test      eax, eax                        ;// Zero, we're compared this byte already, we've found it!
            jz            found

            cmp            [edi], [esi]                  ;// Compare last character
            jne            compare_cleanup                  ;// No match, keep looking

            sub            eax, 1                              ;// Decrement counter (checked on next iteration for zero)
            sub            edi, 1                              ;// Get previous byte
            sub            esi, 1                              ;// Get previous byte

            jmp            compare_loop                  ;// So far, so good, keep checking the needle against the buffer

found:
            mov            eax, edi                        ;// Return current location in buffer
            ret

not_found:
            xor            eax, eax                        ;// Return zero
            ret

to_memchr:
            push      ecx                                    ;// Count
            push      edx                                    ;// Push needle (must be passed as an int)
            push      edi                                    ;// Buffer
            call      memchr                              ;// Call memchr(buffer,value,count)
            ret                                           ;// Return memchr's return value
      }
}

Thanks for your time!

This is actually the code of a colleague of mine, I'm just trying to help him get it working, he's an assembly novice also. He's been talking about posting it to code project when it's all finished, so you could look for the final version there later, if you are interested.
ASKER CERTIFIED SOLUTION
Avatar of dimitry
dimitry

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of Sandra-24
Sandra-24

ASKER

Haven't had time to look over it fully yet, but cmpsb looks like the way to go. What's the .386 do? Executes in some sort of compatibility mode or something? Also is there any difference between inc ecx and add ecx, 1? I'll be back later to try and understand your code more fully.

Thanks,
-Sandra
I give you this example for your information only. 'cmpsb' itself should help you.
.386 is not a command. It is directive to TASM compiler to use 80386 comamnd set.
inc ecx is shorter in code length and more efficient in cpu clock cycles than add ecx, 1. Result is the same.
Excellent, thanks. I think I can make it from here. If I need any more help I will make a new question.

Just as a matter of curiosity, rep command is a sort of a loop with a counter? Is that more effecient than doing the same with an inc (or dec) command, a cmp, and a jz?

There seems to be several different ways of doing a lot of things in assembly, like to test register eax for zero, you can use, cmp eax,0 with jz or test eax,eax with jz, or or eax,eax with jz (i think). How do you know which is better to use? Is it usually true that fewer/less complex commands = faster code?

Thanks again,
-Sandra
rep "converts" any string command to loop until cx-- != 0.
Strings commands are:
  rep movsb, movsw, movsd (move byte, word, dword from ds:[si] to es:[di]) - memcpy in other words
  rep cmpsb, cmpsw, cmpsd (compare byte, word, dword from ds:[si] to es:[di]) - memcmp in other words
There are other commands but those are most useful.
These is also repne that makes "loop" until cx-- !=0 and ZeroFlag == 0. However I suspect that "clever" compiler
will replace rep cmpsb to repne cmpsb automatically.

x86 assembler is pretty "rich" and allows you to do things in different ways. My only recommendation is:
  Write the code in the way you and somebody who will use it may read it. I mean keep it simple.
  For example, despite it may be not 100% efficient, it is much more understandable:
    ; if( AX == 0 ) goto ax_eq_0;
    cmp ax, 0
    je ax_eq_0
 than:
    test ax, ax
    jz ax_eq_0

Good luck !