5
\$\begingroup\$

I'm learning x86 assembly on Linux and this is the first program I wrote, so I'm looking for all kinds of ways to improve it.

; Build using these commands:
;   nasm -f elf printenv.asm
;   ld -o printenv printenv.o -m elf_i386 -s

section .data
section .bss

section .text

global _start

_start:
  pop ecx                 ; load argument count
  lea esp,[esp+ecx*4+4]   ; move past every argument and null ptr

  PopEnv:
  pop esi                 ; get next env variable
  cmp esi,0               ; null means end
  je Exit                 ; exit if null

  ; calculate length
  mov ecx,0ffffffffh      ; max length
  xor eax,eax             ; search for null byte
  mov edi,esi             ; where to look for
  repne scasb             ; search
  dec edi                 ; edi points one past the null
  mov byte [edi],0ah      ; replace null with EOL
  sub edi,esi             ; length
  mov edx,edi             ; set arg for sys_write
  inc edx                 ; include EOL

  ; print env to console
  mov eax,4               ; sys_write
  mov ebx,1               ; stdout
  mov ecx,esi             ; location
  int 80h                 ; call
  cmp eax,edx             ; check return value
  jb ExitFailure          ; error

  jmp PopEnv              ; get another env

  Exit:
  mov eax,1               ; sys_exit
  mov ebx,0               ; EXIT_SUCCESS
  int 80h                 ; make the call

  ExitFailure:
  mov eax,1               ; sys_exit
  mov ebx,-1              ; EXIT_FAILURE
  int 80h                 ; make the call
\$\endgroup\$
1
  • \$\begingroup\$ Hello! Please don't make changes to the original post once it has been reviewed, as that invalidates the current answers. Please see our meta side on performing iterative reviews for more information! \$\endgroup\$
    – syb0rg
    Commented Jul 27, 2016 at 18:02

1 Answer 1

1
\$\begingroup\$

There's always a question of what 'improve' might mean. I don't see anything here that is 'wrong,' so mostly I'm commenting on 'style.' That said, if I were doing a code review, these are some of the things I'd mention:

  • You have lots of very clear comments. That's a good thing, especially in assembler. I can't count the number of times I've read code and thought "What the heck was this guy THINKING?" This can be especially important when the code is wrong: He thinks it's doing x, but the code actually does y, so I am probably safe fixing it.
  • You don't have a comment that describes what the program does. While your SE post has a (misspelled) description, the code does not.
  • While it is not an issue for such a short program, you might want to start thinking in terms of 'routines.' Having a chunk of code that counts the length of a string (or that prints a string) that you can copy into your next program can be useful.
  • While it works, I wouldn't get into the habit of using repne. It's not the best performing way to do this kind of loop (see https://stackoverflow.com/a/34141835/2189500). BTW, this is one of the reasons to use a high level language (like C). Compilers already know all this stuff and (try to) use the best possible construct.
  • I expect that with a bit of fiddling (by using an offset on the mov) you can remove the dec edi/inc edx.
  • I might consider changing the jb ExitFailure. jb can only jump over a certain amount of code, so your existing construct will work even if PopEnv was far away. But since your code is small, you could do:

a

jnb PopEnv              ; get another env

ExitFailure:
mov eax,1               ; sys_exit
mov ebx,-1              ; EXIT_FAILURE
int 80h                 ; make the call

Exit:
mov eax,1               ; sys_exit
mov ebx,0               ; EXIT_SUCCESS
int 80h                 ; make the call

In summary, my comments are mostly just piddly 'style' stuff.

\$\endgroup\$
1
  • \$\begingroup\$ I timed the repne version vs the cmp and your suggestion was about 15% faster. Relocating the ExitFailure block was definitely much better. The only detail is that we should use jnl because the kernel returns negative values to indicate errors. \$\endgroup\$
    – Douglas
    Commented Jul 27, 2016 at 17:57

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.