-1

this is a code to reverse a string I am really struggling to finish this code. I could not find what's wrong with it, however the output is wrong.

    mov esi, OFFSET source             
    mov edi, OFFSET target          
    add edi, SIZEOF target-2
    mov ecx, SIZEOF source-1   

L1:                             
    mov  al, [esi]              
    mov  [edi], al                
    inc  esi                      
    dec  edi                     
    loop L1
Peter Cordes
  • 245,674
  • 35
  • 423
  • 606
  • https://stackoverflow.com/questions/33685870/how-to-reverse-and-print-a-string-in-assembly-language – Ng Sharma Oct 31 '18 at 06:09
  • You can do `mov edi, OFFSET target + SIZEOF target - 2`. Or just put a label at the end of the target buffer, and do `mov edi, OFFSET end_target - 2`. Anyway, what does this code do differently from what you want to happen? Does it write to 1 byte before the beginning of `target` or something? – Peter Cordes Oct 31 '18 at 06:17
  • 2
    @NgSharma, that question has rather, --unhelpful-- answers to say the least. – Johan Oct 31 '18 at 10:50

1 Answers1

1

It's a bit odd that you pass two lengths to the function. All sorts of bad stuff might happen if the two mismatch. Better to pass the length of the string explicitly, or have the code figure out the length.
As getting the length of a old-skool c-string is non-trivial to code, but trivial to google, I'm going to just pass it as a parameter.

'...the output is wrong.'
The problem is that your strings need to be zero terminated, but you are not putting the terminating zero on your destination string.

First off, if you want to have the string be a valid c-style string, make sure you add a terminating zero, like so: source db "test test",0

    mov esi, OFFSET source        ;Start of source
    mov edi, OFFSET target        ;start of dest
    ;length EQU SIZEOF source      ;we are reversing source
    mov ecx, SIZEOF source        ;Length of the string 
                                  ;(includes the terminating 0)

Setup:
    ;//a c-string must have a terminating 0!
    xor eax,eax                   ;al=0, put the terminating zero in first   
L1:                             
    mov  [edi+ecx-1], al          ;if length(ecx)=1, then write to [edi] directly.  
    mov  al, [esi]               
    inc  esi                                           
    loop L1

Remarks on the code
There is no need to keep three counters in flight (edi,esi,ecx), two is enough. esi counts up, ecx counts down.
The x86 has lots of really helpful addressing modes, which are mostly free performance wise.
The last iteration will read the terminating zero in al, we don't need to reverse this and you've already written it at the beginning, so it is (silently) dropped.
Because of the terminating zero, length is at least 1. This is good because if somehow you were to feed 0 into loop it will loop 'forever'; not good ('forever' being 4+ billion times).

Note that your code does not take account of Unicode, so it will not work for UTF8, but lets assume it's just a learning exercise.

If you follow an ABI, then you can just pass the parameters in registers, meaning you can skip on some of the initialization. Given that your code is not going to win any prizes for raw speed, I've skipped this step.

Johan
  • 71,222
  • 23
  • 174
  • 298
  • It's not a function, the SIZEOF operator in MASM is an assemble-time thing for static data. From previous questions, I think the OP has 0-terminated C strings in known-length static buffers, and they're having a hard time reversing the data before the terminator and then just copying the terminator. – Peter Cordes Oct 31 '18 at 13:57
  • @PeterCordes, yeah it's been a while since I used MASM. I was correct about the 0-termination being the problem, thanks for confirming that. – Johan Oct 31 '18 at 14:28
  • length EQU SIZEOF source Is giving me a syntax error along with this line mov ecx, length –  Oct 31 '18 at 17:58
  • @Jinna: use `mov ecx, SIZEOF source`. Or use `length = $ - source` right after you define `source`. (`$` is the current position, so it will calc the wrong size if you put it anywhere else. See [How does $ work in NASM, exactly?](https://stackoverflow.com/q/47494744) for a NASM version. I'm not sure if EQU in MASM is exactly what you want, possibly just a text substitution that will evaluate `$` at the wrong place; I think `=` is definitely evaluated to a number at the position you write it (but is still an assemble-time constant that doesn't take space in the data section).) – Peter Cordes Oct 31 '18 at 22:17