5
\$\begingroup\$

I am a beginner in assembly. But that didn't stop me from making a simple operating system completely made in assembly. It's very simple and doesn't do much. It's just a proof of concept. I would like to know what I can improve. Any feedback would be great.

bits 16
mov dl, 0 ; Cursor position column
mov dh, 0 ; Cursor position row
; Draw a really simple UI
; Draws a blue background
mov ah, 09h 
mov al, 32
mov bl, 10h
mov cx, 1000h
int 10h 
; Draws a gray bar
mov ah, 09h
mov al, 32
mov bl, 70h
mov cx, 160d
int 10h
main:
 ; Waits for a key press
 mov ah, 00h
 int 16h
 ; Compare the al register to see what key the user pressed
 ; If the user pressed the 's' key move the cursor down
 cmp al, 115
 je Down
 ; If the user pressed the 'w' key move the cursor up
 cmp al, 119
 je Up
 ; If the user pressed the 'a' key move the cursor left
 cmp al, 97
 je Left
 ; If the user pressed the 'd' key move the cursor right
 cmp al, 100
 je Right
 ; If the user pressed the Space Bar make the background of the cursor magenta
 cmp al, 32
 je SpaceBar
 ; Jump back to main
 jmp main
 ; Our functions
 SpaceBar:
 mov ah, 09h
 mov bl, 50h
 mov cx, 1d
 int 10h
 jmp main 
 Right:
 add dl, 1
 call SetCursor
 jmp main
 ret
 Left:
 sub dl, 1
 call SetCursor
 jmp main
 ret
 Up:
 sub dh, 1
 call SetCursor
 jmp main
 ret
 Down:
 add dh, 1
 call SetCursor
 jmp main
 ret
 SetCursor:
 mov ah, 02h
 mov bh, 00
 int 10h
 ret
 times 510-($-$$) db 0
 dw 0xAA55
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Feb 25, 2017 at 1:02
\$\endgroup\$
2
  • \$\begingroup\$ Which variant of Assembly are you using? x86 assembly? \$\endgroup\$ Commented Feb 25, 2017 at 1:31
  • \$\begingroup\$ @EBrown I'm really not sure actually. But I would say yes. \$\endgroup\$ Commented Feb 25, 2017 at 2:43

2 Answers 2

3
\$\begingroup\$

Your program uses many BIOS functions that require you to set the display page beforehand in the BH register. You specified it correctly in the SetCursor routine, but not in the several WriteCharacterAndAttribute calls.


mov dl, 0 ; Cursor position column
mov dh, 0 ; Cursor position row

To save space we usually write this pair of instruction as:

xor dx, dx ;Set CursorColumn=0 CursorRow=0

; Draw a really simple UI
; Draws a blue background
mov ah, 09h 
mov al, 32
mov bl, 10h
mov cx, 1000h
int 10h 

Here you need to verify your understanding of the count in CX. It specifies the number of characters that you want to draw. The hexadecimal value 1000h equals 4096 in decimal, but the usual 80x25 textscreen only has 2000 characters to play with! In your program it seems to work OK only because the video memory is much larger than that single display page you're looking at. Nonetheless you should correct it to avoid developing the bad habit of buffer overflows.

; Draw a really simple UI
; Draws a blue background
mov ax, 0920h ;AH=09h Function, AL=" " Character
mov bx, 001Fh ;BH=00h Display page, BL=10h BrightWhiteOnBlue
mov cx, 2000 ;Character count
int 10h

As you can see I combined a pair of settings in a single instruction. This saves space, can be a bit faster, and the comments make it absolutely clear what is going on.

The same applies to drawing the gray bar, although it's not clear:

  • why you wrote 160d with the d suffix to specify decimal, as decimal is the default in the absence of any prefix/suffix.
  • what you expect to obtain with a counter of 160. It should give you a 2-rows gray bar where you might have expected a 1-row gray bar.

When it comes to the compairing section of the program you could definitely tidy up a bit. It will enhance readability a lot.

  • Write the comments as tail-comments and keep them aligned
  • Write short(er) comments that convey all that is needed
  • Replace the ASCII codes by their actual characters
  • Don't use double interlineation. (This one is debatable!)

So:

cmp al, 's' ; The 's' key moves the cursor down
je Down
cmp al, 'w' ; The 'w' key moves the cursor up
je Up
cmp al, 'a' ; The 'a' key moves the cursor left
je Left
cmp al, 'd' ; The 'd' key moves the cursor right
je Right
cmp al, ' ' ; The ' ' key makes the background of the cursor magenta
je SpaceBar
jmp main ; Jump back to main

 je SpaceBar
 jmp main
SpaceBar:

In a construct like this most programmers will try to save some bytes by using the opposite conditional jump and else fall through:

 jne main
SpaceBar:

In this simple program it is what I would do. If the polling section were much longer I would keep what you wrote.


Right:
 add dl, 1
 call SetCursor
 jmp main
 ret

The ret instruction below a jmp is totally useless. The CPU will never reach it. Now it just consumes a byte and is testament of (lack of) skills.


In its present form your program entirely depends on the values in DL and DH having a permanent meaning. If your program were to become longer and more complex, this would no longer be a desirable approach. Then variables would be in order. An example on how to use these:

CursorCol db 0
CursorRow db 0
 ...
 add CursorCol, 1
 ...
SetCursor:
 mov dl, CursorCol
 mov dh, CursorRow
 mov bh, 0
 mov ah, 02h
 int 10h
 ret

As a final note on the indentations that you used. I've nothing against it as long as it is used in a consistent manner. This seems not to be the case! Sometimes the editor on StackExchange/CodeReview is to blame though.

answered Feb 26, 2017 at 16:38
\$\endgroup\$
3
\$\begingroup\$

Eventually you're going to want to handle other key-presses, correct? You might also want to handle other encoding styles, like EBCDIC, correct?

So, I've never done Assembly before, but these are pretty simple things to design for, and I hope the implementation is correct. First, we want to build a (very large) jump table for ASCII. Essentially, a jump table eliminates all of your comparisons down to exactly one comparison. It's pretty easy to set up:

 goto ascii(al) ; Jump to the line of the character code
ascii: goto nulInput ; This would be `0` code input
 goto sohInput ; This would be `1` code input / Start of Heading
 goto stxInput ; This would be `2` code input / Start of Text
 ; Add End of Text (3), End of Transmission (4), Enquiry (5)
 ; Acknowledgement (6), Bell (7)
 goto bsInput ; This would be `8` code input / Backspace
 goto htInput ; This would be `9` code input / Horizontal Tab
 goto lfInput ; This would be `A` code input / Line Feed
 ; Add Vertical Tab (B), Form Feed (C)
 goto crInput ; This would be `D` code input / Carriage return
 ; Shift Out (E), Shift In (F), Data Link Escape (10)
 ; Device Control 1 / XON (11), Device Control 2 (12)
 ; Device Control 3 / XOFF (13), Device Control 4 (14)
 ; Negative Acknowledgement (15), Synchronous Idle (16)
 ; End of Transmission Block (17), Cancel (18), End of Medium (19)
 ; Substitute (1A)
 goto escInput ; This would be `1B` code input / Escape
 ; File Separator (1C), Group Separator (1D), Record Separator (1E)
 ; Unit Separator (1F)
 goto spacInput ; This would be `20` code input / Space
 goto exclInput ; This would be `21` code input / Exclamation Point
 ; In Order:
 ; " # $ % & ' ( ) * + , - . / 0 1 2 3 4 5 6 7 8 9 : ; < = > ?
 ; @ A B C D E F G H I J K L M N O P Q R S T U V W X Y Z [ \ ] ^ _
 ; ` a b c d e f g h i j k l m n o p q r s t u v w x y z { | } ~
 goto delInput ; This would be `7F` code input / Delete

So you should get the point, go through the ASCII spec line-by-line and put a line for each record in the spec, then a label for each of those lines. For the ones you don't wish to implement yet, simply have a restart label or badCode label that ignores the input.

Once that's done we now know how to implement EBCDIC, just build an EBCDIC jump table.

What you might not realize is that having 128 (because eventually you want to support all codes/keys, right?) cmp statements will cause negative performance because al is compared up to 128 times for each key press, that's a lot. So we build a jump table that might be more work to implement initially, but in the end we'll end up with a much easier to maintain setup, and performance will not degrade. When we call jmp ascii(al), it jumps to the ascii label, then to the line identified by al. Easy stuff.


Regarding your indentation and such, from what I've seen usually assembly is spaced fairly far from the left (8-12 spaces) then all labels go at position 0. This helps keep them in plain sight, since assembly doesn't have 'functions' and such, and there's not a real concept of 'nesting' (you can nest labels, but any label can be accessed from any other location/scope so it doesn't always do any good).

answered Feb 25, 2017 at 1:57
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.