How to check stdin properly

[aVoX]

Hello there!
I've written a small 08/15 guess-the-number program using NASM and the MS VC Runtime. Using scanf I achieved to fetch inputs from the console. The inputs are checked with CMP/JL/JG and so on but if the user enters one or more characters instead of a number, my program infinitely writes a message ('guess' in the following code) to the output. (Even though I checked scanf's return value)

Here is the "critical" part of the code:

Code: [Select]

.loop_head:
PUSH guess
CALL _printf
ADD esp, 4

PUSH dword in
PUSH fmt
CALL _scanf
ADD esp, 8

CMP eax, 0
JLE .loop_head
; [ . . . ]
[segment .data]
guess: db "Guess a value between 1 and 100: ", 0
fmt: db "%d", 0

So what exactly did I do wrong?

Thanks in advance.

[Frank Kotler]

Untested - I'll try to get to it. I suspect that you may need to flush stdin, in the case of a match failure.

Later,
Frank

[aVoX]

I'll just leave this here: http://db.tt/hYnHX7XB
I know it's not a well written code, but this originates from my first tries with NASM
(Note: To run the build script successfully you'll need a msvcrt.lib in your %PATH% and a VS2010 installation)

[Frank Kotler]

Well, my hunch was wrong. Calling fflush (not flush) on stdin does not solve the problem. Research continues...

I did notice one bug. Loading "rnd" with a 32-bit value and then reloading it with a 16-bit value leaves the upper bits untouched (potentially non-zero). This results in "The number is greater than ..." for guesses far above 100! Easiest thing is probably to replace the 16-bit section (dividing by 100) with 32-bit code (untested)...

More to come... Input from a better C programmer than I welcome!

Best,
Frank

[codeFoil]

I'm not much of a C programmer here, but I think Mr. Kotler was on the right track in his first response.

From a draft copy of the C standard concerning the fscanf group of functions:

An input item is read from the stream, unless the specification includes an n specifier. An
input item is defined as the longest sequence of input characters which does not exceed
any specified field width and which is, or is a prefix of, a matching input sequence.251)
The first character, if any, after the input item remains unread. If the length of the input
item is zero, the execution of the directive fails; this condition is a matching failure unless
end-of-file, an encoding error, or a read error prevented input from the stream, in which
case it is an input failure.

However, as noticed, fflush does not do anything on an input stream when the last operation was an input.
Perhaps a call to scanf to read a single character could be used to discard the rest of the line buffer input?

[Frank Kotler]

Yeah, that helps. Thanks, codeFoil! Now it only repeats "Guess..." the number of times as the number of non-digits I enter. Maybe loop back only to a ".flush_stdin:" label instead of all the way back to ".loop_head:"...?

What good is "fflush()" if it doesn't? This is why I "like" asm better!

More,
Frank

[codeFoil]

The scanf minilanguage also includes a no assignment modifier, so I tried flushing the text line by parsing a string of unspecified width.

TestRead.asm

Code: [Select]

extern _scanf, _printf
global _TestRead

section .text


FlushLine:
    PUSH DISCARD_LINE                   ; String "%*s"
    CALL _scanf
    ADD  ESP, 4

; Simply reads and writes an integer, retrying until
; input is valid.
; Returns either an error code from scanf
; or the number of characters written by printf
_TestRead:
    PUSH dword inValue
    PUSH IN_FMT                         ; String "%d"
    CALL _scanf
    ADD  ESP, 8                         
   
    CMP EAX, 0
    JZ FlushLine
    JL InputError

    PUSH dword [inValue]
    push OUT_FMT                        ; String "%d\n"
    call _printf                        ; Return status of printf
    ADD  ESP, 8

Return:
    RET

InputError:
    PUSH EAX                            ; Save error code
    PUSH dword ERR_MESSAGE              ; "Error"
    PUSH ERR_FMT                        ; "%s"
    CALL _printf
    ADD  ESP, 8
    POP  EAX                            ; Return error code
    JMP  Return


section .data
IN_FMT       db "%d"   , 0 , 0
OUT_FMT      db "%d"   , 13, 10, 0, 0, 0, 0
ERR_FMT      db "%s"   , 0 , 0
ERR_MESSAGE  db "Error", 0 , 0, 0
DISCARD_LINE db "%*s"  , 0


section .bss

inValue resd 1
Test.c

Code: [Select]

int TestRead();

int main() {
    return TestRead();
}

I tested this with MS C/C++ compiler from VS 2010, and it seems to work.

I've never liked the printf and scanf routines, and I think I must be missing something really obvious.  C programmers must have a better way than this.

[Frank Kotler]

Back when I was attempting to write C programs, before I got "distracted" by asm, I didn't have much trouble with printf, but found scanf extremely unfriendly! I used to use gets (Shoot me! I didn't know any better!) followed by atoi or atof to avoid it. Learning to use it properly is probably a better solution.

I thought of using %s to discard bad (non-decimal) input, but was afraid of buffer overflow. I'm not sure where the input "goes" with your method. Man-page says "undefined", but... isn't scanf going to use whatever's on the stack as a buffer? I guess it's okay, but I'm not 100% comfortable with it. (oh, that's what that '*' does, I guess)

I'm not too happy with my latest attempt, either. I should state that I haven't got VS of any version (stubborn, I am) so I'm doing it in Linux. Mostly this involved removing underscores. Nasm will put 'em back with "--prefix _" (--postfix _" for OpenWatcom). The other issue is that bash doesn't know "pause", causing(?) a segfault at the end. Oh, wait! The segfault is from exiting with "ret 8". I was linking it without the "startup" code, exits okay with it...

Speaking of "whatever's on the stack", "tries" is used uninitialized. "inc dword [tries]" assumes zero, which may be true in Windows, but was reporting some improbable (negative) number of tries for me. Should probably explicitly initialize it, even for Windows.

I'll post what I've got so far, as is, but it isn't "right" yet!

Code: [Select]

BITS 32

[segment .text]
        [global main]

        ; msvcrt.lib
EXTERN  scanf
EXTERN printf
EXTERN rand
EXTERN srand
EXTERN time
EXTERN system

%define rnd ebp-4
%define tries ebp-8

%macro random 0
PUSH 0
CALL time
ADD esp, 4

PUSH eax
CALL srand
ADD esp, 4

CALL rand
%endmacro

main:
        ENTER 8, 0

random

MOV [rnd], eax

MOV eax, [rnd]
XOR edx, edx
MOV ebx, 100
DIV ebx
ADD edx, 1
MOV [rnd], edx

; INC dword [tries]
; used uninitialized!
mov dword [tries], 1

.loop_head:
PUSH guess
CALL printf
ADD esp, 4

PUSH dword in
PUSH fmt
CALL scanf
ADD esp, 8

cmp eax, 1
jz .good

.flush_stdin:
PUSH dword in
PUSH charfmt
CALL scanf
ADD esp, 8
cmp byte [in], 10 ; LF - works for Linux!
jnz .flush_stdin

jmp .loop_head

.good:

MOV eax, dword [in]

CMP [rnd], eax
            JL      .less
            JG      .greater

JMP .loop_foot

.less:
PUSH dword [in]
PUSH less
CALL printf
ADD esp, 8
ADD dword [tries], 1
JMP .loop_head

.greater:
PUSH dword [in]
PUSH greater
CALL printf
ADD esp, 8
ADD dword [tries], 1
JMP .loop_head

        .loop_foot:

        PUSH dword [tries]
        PUSH won
        CALL printf
        ADD esp, 8

        PUSH pause ; Linux doesn't know "pause"!
        CALL system
        ADD esp, 4

            LEAVE
            RET 8


[segment .data]
guess: db "Guess a value between 1 and 100: ", 0
less: db "The number is lower than %d", 10, 13, 0
greater: db "The number is greater than %d", 10, 13, 0
won: db 10, 13, "You found the number in %d tries!", 10, 13, 0
fmt: db "%d", 0
charfmt db "%c", 0
pause: db "pause", 0

[segment .bss]
in: resd 1 ; user input

Best,
Frank

[codeFoil]

Code: [Select]

.flush_stdin:
PUSH dword in
PUSH charfmt
CALL scanf
ADD esp, 8
cmp byte [in], 10 ; LF - works for Linux!
jnz .flush_stdin


Might be less overhead as:

Code: [Select]

.flush_stdin:
CALL getchar
CMP eax, 10 ; LF would work for MS as well,
                 ; unless some joker embeds a LF
                 ; in an input line. (who would do that?)
jnz .flush_std


I guess this would be equivalent to

Code: [Select]

while (getchar() != 10);
The only problem is that if stdin has been redirected, an end of file error will cause an infinite loop with either version.

assumes zero, which may be true in Windows

It is not.

[Frank Kotler]

You're quite correct that not checking for error is... an error. I considered "getchar" or "getch" or "getc" - I'm never sure which is a "real" function and which is a macro hiding something else. Figured I "had" scanf, so I used it...

Speaking of macros, aVoX only uses "random" once, so it's fine. Unless I'm mistaken, we're only supposed to call srand once, and rand (repeatedly, if necessary) thereafter. Doubt if it would make a difference to a "guess the number" program. Another one of those C factoids that I'm nor really sure about...

Your input on this is much appreciated!

Best,
Frank

[Bryant Keller]

This has been tested and works on Linux, should also work on Windows (move the comment from line 19 to line 20 to adjust the newlines).

A neat feature of fflush is the "NULL argument". The C standard specifies that a NULL argument to fflush should cause a flush of all handles whose last operation was not an input.

Code: [Select]

;; filename: guess.asm
;;
;; assemble (windows): nasmw -f win32 -prefix=_ guess.asm
;; compile (windows): link /subsystem:console guess.obj msvcrt.lib
;;
;; assemble (linux): nasm -f elf guess.asm
;; compile (linux): gcc -o guess guess.o

BITS 32

GLOBAL main
EXTERN printf
EXTERN fflush
EXTERN scanf
EXTERN rand
EXTERN srand
EXTERN time

;%define NEWLINE 13, 10 ; WINDOWS
%define NEWLINE 10 ; LINUX

%macro ccall 2-*
%push _ccall_
%define %$proc %1
%rep %0-1
%rotate -1
push %1
%endrep
call %$proc
add esp, (%0-1) * 4
%pop
%endmacro

SECTION .text

main: push ebp
mov ebp, esp
sub esp, 8

%define tries (ebp - 4)
%define solution (ebp - 8)

;; Generate Random Number

ccall time, 0
ccall srand, eax
call rand
xor edx, edx
mov ebx, 100
div ebx
inc edx
mov [solution], edx

;; Initialize Attempt Counter

xor eax, eax
inc eax
mov dword [tries], eax

;; Get a Number

.again:
ccall printf, guess
ccall fflush, dword 0
ccall scanf, fmt, input
cmp eax, 0
jle .again

mov eax, [input]
cmp [solution], eax

jl .less
jg .greater
jmp .equal

;; Too low, try again.

.less:
ccall printf, less, dword [input]
add [tries], dword 1
jmp .again

;; Too high, try again.

.greater:
ccall printf, more, dword [input]
add [tries], dword 1
jmp .again

;; You win!

.equal:
ccall printf, won, dword [tries]

leave
ret

SECTION .data

guess: db "Guess a value between 1 and 100: ", 0
less: db "The number is lower than %d.", NEWLINE, 0
more: db "The number is greater than %d.", NEWLINE, 0
won: db NEWLINE, "You found the number in %d tries!", NEWLINE, 0
fmt: db "%d", 0

SECTION .bss

input: resd 1
I hope this works. Btw, I changed the names of a few labels to make things line up better in Emacs.

Regards,
Bryant Keller

[Frank Kotler]

Sorry to say... this has the same problem as aVoX originally reported. Enter "abc" and it loops infinitely. It's that "whose last operation was not an input" that's killin' us. If we could eliminate those pesky users, this would be a lot easier!

Best,
Frank

[codeFoil]

I'm never sure which is a "real" function

Yes.  That one bit me.  Microsoft's library implements is as a separate function, but its not standard.

[Bryant Keller]

I totally didn't even notice the 'abc' issue.

This version works by using fgets to read a line, then using sscanf on the buffer. Quick note of warning though, entering values like '10abc19' will be treated as 10 (sscanf catching the first %d in the string).

Code: [Select]

;; filename: guess.asm
;;
;; assemble (windows): nasmw -f win32 -prefix=_ guess.asm
;; compile (windows): link /subsystem:console guess.obj msvcrt.lib
;;
;; assemble (linux): nasm -f elf guess.asm
;; compile (linux): gcc -o guess guess.o

BITS 32

GLOBAL main
EXTERN stdin
EXTERN stdout
EXTERN printf
EXTERN fflush
EXTERN fgets
EXTERN sscanf
EXTERN rand
EXTERN srand
EXTERN time

;%define NEWLINE 13, 10 ; WINDOWS
%define NEWLINE 10 ; LINUX

%macro ccall 2-*
%push _ccall_
%define %$proc %1
%rep %0-1
%rotate -1
push %1
%endrep
call %$proc
add esp, (%0-1) * 4
%pop
%endmacro

SECTION .text

main: push ebp
mov ebp, esp
sub esp, 8

%define tries (ebp - 4)
%define solution (ebp - 8)

;; Generate Random Number

ccall time, 0
ccall srand, eax
call rand
xor edx, edx
mov ebx, 100
div ebx
inc edx
mov [solution], edx

;; Initialize Attempt Counter

xor eax, eax
inc eax
mov dword [tries], eax

;; Get a Number

.again:
ccall printf, guess
ccall fflush, dword [stdout]
ccall fgets, buffer, dword 255, dword [stdin]
ccall sscanf, buffer, dfmt, input
cmp eax, 0
jle .again

mov eax, [input]
cmp [solution], eax

jl .less
jg .greater
jmp .equal

;; Too low, try again.

.less:
ccall printf, less, dword [input]
add [tries], dword 1
jmp .again

;; Too high, try again.

.greater:
ccall printf, more, dword [input]
add [tries], dword 1
jmp .again

;; You win!

.equal:
ccall printf, won, dword [tries]

leave
ret

SECTION .data

guess: db "Guess a value between 1 and 100: ", 0
less: db "The number is lower than %d.", NEWLINE, 0
more: db "The number is greater than %d.", NEWLINE, 0
won: db NEWLINE, "You found the number in %d tries!", NEWLINE, 0
dfmt: db "%d", 0
sfmt: db "%[256]s", 0

SECTION .bss

input: resd 1
buffer: resb 256