Skip to main content
fixed typo
Source Link
Edward
  • 67.3k
  • 4
  • 120
  • 284

But you could easily restructure that to eliminate the undonditionalunconditional jmp like this:

But you could easily restructure that to eliminate the undonditional jmp like this:

But you could easily restructure that to eliminate the unconditional jmp like this:

added update
Source Link
Edward
  • 67.3k
  • 4
  • 120
  • 284

#Update: On my machine (quad core running 3.2GHz under 64-bit Linux) I found this version to be slightly (15%) faster than the corresponding repnz scasb version:

strlen2:
    mov edx, [esp + 4]
    xor eax,eax ; count = 0
    jmp overit
looptop:
    inc edx
    inc eax
overit:
    cmp byte[edx],0
    jnz looptop
    ret
    

#Update: On my machine (quad core running 3.2GHz under 64-bit Linux) I found this version to be slightly (15%) faster than the corresponding repnz scasb version:

strlen2:
    mov edx, [esp + 4]
    xor eax,eax ; count = 0
    jmp overit
looptop:
    inc edx
    inc eax
overit:
    cmp byte[edx],0
    jnz looptop
    ret
    
Source Link
Edward
  • 67.3k
  • 4
  • 120
  • 284

I have a number of comments that I hope you find helpful.

Code comments

First, your code is generally well commented and I had little difficulty in understanding what the code was doing or why. That's a good thing. A few minor points, though. First, your comment for the overall function says

; input register: EAX - the string

however, it's probably useful to point out that EAX is actually a pointer to the string and isn't intended to contain a whole string.

Also, comments should tell something not obvious about the code. So this:

        jmp     _NextBlock; repeat loop

is arguably OK, but this:

        inc eax ;increment EAX

is not really helpful. Instead you should say why the code is incrementing EAX rather than simply repeating what the instruction does.

Structure

Overall, the structure was OK, but the line

        jmp _NextBlock ; go to _NextChar

near the top of the function is not needed, since _NextBlock is the next instruction anyway. The same is true of the jmp just before _FinalizeFinalBlock. Generally speaking, you should strive to eliminate unconditional jumps. So for example, your code includes this:

    _FinalizeFinalBlock:
        cmp     byte[eax], NULL_TERMINATOR ;Compare the characters of the final block to the null byte.
        jz      _Exit ;if this byte is null, proceed to _Exit
        ; if not...
        inc eax ;increment EAX
        jmp     _FinalizeFinalBlock ;repeat loop

    _Exit:

But you could easily restructure that to eliminate the undonditional jmp like this:

        dec eax ;   
    _FinalizeFinalBlock:
        inc eax ;increment EAX
        cmp     byte[eax], NULL_TERMINATOR ;Compare the characters of the final block to the null byte.
        jnz     _FinalizeFinalBlock ; keep going until final block is NULL

    _Exit:

Algorithm

The most significant problem I see with the code is the algorithm that relies on strings having multiple trailing NUL bytes. This may be true for code that calls this function, but it's not generally true of C strings which typically end with only a single NUL character.

Instruction set use

The repnz scasb instruction is probably going to be a lot more efficient that the loop you've constructed. I'd recommend reading about it and then incorporating that into your code.

Register use

The ebx register is destroyed and not saved by the function. This may be fine for your purposes, but it should be documented in comments at the top of the function.

Also, the code doesn't actually alter ebp so there isn't any point to pushing it onto the stack and popping it off at the end of the routine. (Maybe you meant ebx?)