7
\$\begingroup\$

I am learning to code in the Assembly language for the x86 architecture. I currently know of a few registers and certain arithmetic operations like add, sub, inc, dec, mov and to check for conditionality using certain jump statements. I am using some "ready-baked" code for input and output. The input - call_hex reads hex from the input and stores it in the eax register and the output - print_eax just prints whatever is in the eax. I am no programmer per se, my end goal is to understand enough assembly to start picking up other things like exploit development. The code below is an exercise to print prime numbers. I know the code I have below could look downright ugly. But if you were rating the code, how much would I get on a scale of 1 to 10, 10 being the highest.

; A prime number is an integer that is divisible only by 1 and by itself.
; A program that takes a number n as input, and prints back to the console all the    primes that are larger than 1 but not larger than n.

format PE console
entry start

include 'win32a.inc' 

; ===============================================
section '.text' code readable executable

start:
    ; Your program begins here:

    ; Division Operations - 
    ;eax <- edx:eax/arg
    ;edx <- edx:eax % arg

    call    read_hex

lb2:    
    dec     eax
    jz      exit
    jmp     prime_check

prime_check:

    ; The check for whether 2 is a prime is done separately here
    mov     ecx, eax
    sub     ecx, 2
    jz      is_prime

    mov     ecx, eax        ; eax will be overwritten, hence need to store it someplace else to recover it
    mov     edx, 0          ; initialing the edx:eax registers
    mov     ebx, 2

lb1:
    dec     eax             ; The 3 lines of code are there cause I need a way to exit the program, when eax inside this block is 1.
    jz      exit            ; This seems to be an easy fix. Although downright inelegant
    inc     eax
    div     ebx             
    mov     eax, ecx
    sub     edx, 1
    jc      lb2             ; The case where number isnt prime
    mov     eax, ecx        ; re-initialing the edx:eax registers
    mov     edx, 0          ; re-initialing the edx:eax registers
    inc     ebx
    sub     ecx, ebx
    jz      is_prime
    mov     ecx, eax    
    jmp     lb1

is_prime:
    call    print_eax
    jmp     lb2

exit:
    ; Exit the process:
    push    0
    call    [ExitProcess]

include 'training.inc'
\$\endgroup\$
5
  • 3
    \$\begingroup\$ "how much would I get on a scale of 1 to 10, 10 being the highest." -- We don't rate code around here. We make it better ;-) \$\endgroup\$
    – janos
    Commented Jan 15, 2015 at 16:30
  • \$\begingroup\$ The check for whether 2 is a prime is done separately here - what do you mean? 2 is a prime. \$\endgroup\$
    – Jamal
    Commented Jan 15, 2015 at 16:32
  • \$\begingroup\$ @Jamal Its just checking to see if the number in the eax register is 2 or something else. If it's something else, I do the prime test elsewhere. If it's 2 I don't do any more tests on it. \$\endgroup\$ Commented Jan 15, 2015 at 16:42
  • \$\begingroup\$ Oh, okay. That comment sounded a bit misleading. \$\endgroup\$
    – Jamal
    Commented Jan 15, 2015 at 16:44
  • \$\begingroup\$ @janos Ok. So would you say the code is okay or can it be vastly improved. \$\endgroup\$ Commented Jan 15, 2015 at 16:47

1 Answer 1

2
\$\begingroup\$

This code can be much improved.

The jmp prime_check is superfluous.

The comment ; The check for whether 2 is a prime is done separately here is wrong because it actually jumps to is_prime when n=3 due to the dec eax used at lb2. Also checking for 2 is usually done with cmp eax,2 and je is_prime.

If you cleared EDX (with xor edx,edx) after lb1: you wouln't have to write it 2 times.

Why don't you replace that downright inelegant code with cmp eax,1 and je exit?

The usual way to test if EDX is zero is to use test edx,edx and jz lb2.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.