-6
void encrypt_chars(int length)
{
    char temp_char;                 

    for (int i = 0; i < length; i++)    
    {
        temp_char = OChars[i];          

        __asm {                         
            push   eax                  
            push   ecx                  

            movzx  ecx, temp_char       
            call   encrypt_nn           
            mov    temp_char, al        

            pop    ecx                  
            pop    eax                  
        }
        EChars[i] = temp_char;          
    }
    return;


    __asm {

    encrypt_nn:
        mov eax, ecx        
            inc eax 
            ret
    }

The encryption part is fine, but I thought I would copy and paste the code to the decryption and instead of incrementing I would decrement the values, so that it would go back and decrypt the data


//---------------------------------------------------------------------------------------------------------------
//----------------- DECRYPTION ROUTINES -------------------------------------------------------------------------
//
void decrypt_chars(int length)
{
    char temp_char;                     

    for (int i = 0; i < length; i--)
    {
        temp_char = OChars[i];

        __asm {                     
            push   eax                  
            push   ecx                  

            movzx  ecx, temp_char       
            call   encrypt_nn           
            mov    temp_char, al        

            pop    ecx                   
            pop    eax              
        }
        EChars[i] = temp_char;      
    }
    return;
}
Bhargav Rao
  • 50,140
  • 28
  • 121
  • 140
  • 3
    This is assembly language not c++ – Ed Heal Dec 22 '17 at 20:07
  • 6
    What's your question? – R Sahu Dec 22 '17 at 20:07
  • so basicly you just take a character from an array and increment it by 1, then store the last half back into the array? that's not a very good encryption. basicly you would just have to dec ecx and you had the char back [edit: isn't this called "cesar's encryption"?] – clockw0rk Dec 22 '17 at 20:12
  • yes I am trying to do cesar's encryption but I am not very good at assembly language. – Mathswork help Dec 22 '17 at 20:32
  • 1
    Thanks I just noticed some of my mistake – Mathswork help Dec 22 '17 at 20:35
  • Using a function call from inline asm seems crazy here. You could write the char-increment and char-decrement loops in inline asm is about as many instruction. Or you could write one function that uses +1 or -1 in a register, i.e. `add byte [mem], al` instead of `inc` / `dec`, so it's the same loop but adding a runtime variable number. This is actually just as efficient. https://stackoverflow.com/questions/36510095/inc-instruction-vs-add-1-does-it-matter. – Peter Cordes Dec 22 '17 at 21:06
  • Your edit turned this into a totally separate question, which invalidates the answer. (For the right answer (`lea`), ask a compiler to compile `int foo(int a) { return a+4; }` with a register-arg calling convention: https://godbolt.org/g/AR5EnG. – Peter Cordes Dec 23 '17 at 16:16
  • If you have another question, ask it separately instead of editing to change this into a very different question. Anyway, you know you can `add eax, 4` instead of `inc eax` 4 times, right? – Peter Cordes Dec 23 '17 at 20:46

1 Answers1

3

Answer:

In order to do the opposite of your encryption operation, you should replace the call to encrypt_nn with a new routine that decrements instead of increments:

 __asm {

    decrypt_nn:
        mov eax, ecx        
            dec eax 
            ret
    }

Comment:

You have changed the loop statement from

for (int i = 0; i < length; i++)  

to

for (int i = 0; i < length; i--)  

In the first case, loop will iterate through all values between 0 and length-1. This means you will iterate through your arrays of chars (assuming length is its size).

In the second case, you will get unpredictable behavior, since you are testing for i < length but doing i-- in each loop iteration. See more information here.

Yaniv Shaked
  • 698
  • 6
  • 12
  • Actually it's unpredictable exactly what will happen with `i--` (unless you checked what MSVC actually does for this code), because [signed overflow is undefined behaviour in C++](http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html). And the compiler can notice at compile time that it's inevitable if `length > 0` on function entry. It could emit an illegal instruction so this function faults if it's ever executed. Or it could emit code that assumes `length` must be non-positive, so the loop runs zero times. i.e. UB implies a range limit. – Peter Cordes Dec 22 '17 at 20:41
  • Thanks for the enlightenment, I will fix my post – Yaniv Shaked Dec 22 '17 at 20:56