0

Recently I decided, it would be funny, to code some simple MSDOS game. Needles to say, I need code for handling keyboard events.

This is what I came up with for testing:

int i, c = 0;
for ( i = 0; i < 10; i++ )
{
    asm
    (
        "mov $0x00, %%ah         \n"
        "mov $0x00, %%al         \n"
        "int $0x16               \n"
        //"jnz keydn             \n"
        //"mov $0x00, %%al       \n"
        //"keydn:                \n"
        "movw %%ax, (%0)         \n"
            : "=r"(c)
    );

    printf( "%d\n", c & 0xFF );
}

The code is supposed to wait for keypress, and then print out ASCII value of the character. And everything works as expected unless I press key like backspace or esc - then segmentation fault occurs.

enter image description here

I'm unfamiliar with assembly, but really I can't figure out what may cause this error.

I compile with djgpp, and run executables in DosBox

Everything is based on information provided here:

Thank you in advance! :)

Jacajack
  • 759
  • 2
  • 11
  • 23
  • 1
    `"mov $0x00, %%ah\n" "mov $0x00, %%al\n"` could be replaced and done as `"xor %%ax, %%ax\n"` or better yet is use an input operand to pass `0` into the template (the register `a` can become an input and output operand). This would eliminate the need for the template itself to zero out the _AX_ register. – Michael Petch Jun 23 '16 at 17:46
  • @MichaelPetch Right, I understand how it works, but as I said before, I've never used bare assembly before, so I wanted to keep everything as simple as possible. Anyway, does it improve execution speed, or something? – Jacajack Jun 23 '16 at 17:49
  • Yes, and if you writing for constrained memory environments takes fewer bytes of memory to encode. – Michael Petch Jun 23 '16 at 17:50
  • @MichaelPetch That's good to know, sir. Thank you :) – Jacajack Jun 23 '16 at 17:51
  • Assembler templates are very tricky to write PROPERLY, and especially since you are a new comer to assembly makes it even more complicated. I'd usually recommend writing full functions in assembly and call them from _C_. This would free you from dealing with the intricacies of inline assembler. The problem would be that such code would likely be less efficient. – Michael Petch Jun 23 '16 at 18:07
  • 1
    @MichaelPetch I understand. From my today's experience I can see, that even assembly basics are pretty tricky, though. Anyway, I don't think I need some great performance in this case, at least in keyboard handler, since system timer, that I use, runs on 18.2Hz, which is really slow. When it's going run on real PC from a floppy disk, I'm going to treat whole thing as a success ;) – Jacajack Jun 23 '16 at 18:19
  • You could have written code like `asm ( "int $0x16\n" : "=a"(c) : "a"(0) );` . We use `a` as both an input and output operand. The template will make sure _AX_ is zero upon entry. The result will be returned in `ax` which will then be moved to variable `c`. I'd suggest compiling with optimizations on (Higher than `-O0`, like `-O1`, `-O2` etc). GCC should be able to optimize the variable `c` to simple register usage. If compiling to conserve space (possible with 16-bit code) by sacrificing some speed you could always use optimization level of `-Os` – Michael Petch Jun 23 '16 at 18:57
  • Probably not a bad idea to use `asm volatile` in my last example. If you don't use the return value (not the case in your code snippet) the entire thing could be optimized away when it might be useful to use the interrupt to wait for a keystroke without caring about storing it anywhere. In essence the template has a side effect (waiting for keystroke) that you may wish to never be removed. – Michael Petch Jun 23 '16 at 20:18
  • @MichaelPetch I wanted one function that waits for keypress, and one that doesn't. Unfortunately there was a lot of weird behavior in my functions, I didn't want - and at this moment I realized I completely forgot about `conio.h` which has its `kbhit( )` and `getch( )`. Although I think asm solution would be faster, this solution is suitable for me right now. By the way, I tried to run my game on `Compaq LTE 5380`, and unfortunately I failed. I guess that is because I'm writing graphics data directly to memory, instead of using BIOS functions. I'm gonna see what what I can do about that later – Jacajack Jun 23 '16 at 20:30
  • Oh I completely forgot that [DGCPP](http://www.delorie.com/djgpp/doc/libc/libc_2.html) had a `conio` library. Although you'd pay a bit of overhead to call a function to get a keystroke, I think that may be a small price to pay to ensure the code works. Small overhead to avoid inline assembler seems like a bonus. – Michael Petch Jun 23 '16 at 20:34
  • @MichaelPetch What do you mean exactly? – Jacajack Jun 23 '16 at 20:38
  • Since you are new to assembler, and already demonstrated that you have problems with using inline assembler - it may be a better idea to call the conio routines to get a keyboard key stroke. If you can do it in _C_ with a library call, that is far easier than learning the intricacies of inline assembler. If you get inline assembler wrong (which is easy to do) you run the risk of creating hard to find bugs as your program grows. I'm offering the advice - If you can use _C_, and library calls to get the job done - do it. – Michael Petch Jun 23 '16 at 20:42
  • In this case I see DJGPP also has a thin wrapper around the BIOS int 16h handler too: http://www.delorie.com/djgpp/doc/libc/libc_73.html . Part of their BIOS library. – Michael Petch Jun 23 '16 at 20:46
  • @MichaelPetch Oh, right, that's what I think too. Anyway, assembler seems to be useful to know, so I'll probably try to practice it a little bit. PS: That DGCPP wrapper looks pretty interesting. – Jacajack Jun 23 '16 at 20:50
  • 1
    this question: has several valid, portable answers to your question. – user3629249 Jun 24 '16 at 22:12
  • @user3629249 I know how to use `conio.h`, but anyway, thank you :) – Jacajack Jun 24 '16 at 22:14

1 Answers1

3

This is certainly broken: movw %%ax, (%0): "=r"(c) It tries to write into memory at address given by operand 0 which is an output operand and as such uninitialized. Also it's not a pointer. You probably want to do something like:

   asm
    (
        "mov $0x00, %%ah         \n"
        "mov $0x00, %%al         \n"
        "int $0x16               \n"
            : "=a"(c)
     );

PS: learn to use a debugger or at least cross-reference the register dump with your code.

Jester
  • 56,577
  • 4
  • 81
  • 125
  • 1
    Thank you! It solved the problem, works like a charm now :) Unfortunately, I'm not familiar with DOS debuggers, and stuff, I'll try to improve my pathetic assembly skills, though – Jacajack Jun 23 '16 at 16:16