From c541d5fa08f0a631915547e4489048163680b240 Mon Sep 17 00:00:00 2001
From: Andrew Ekstedt <andrew.ekstedt@gmail.com>
Date: Wed, 24 May 2023 22:26:50 -0700
Subject: [PATCH] lib/flags: more progress + test script

---
 .gitignore    |  1 +
 flagtest.s    | 18 ++++++++++---
 lib/flags.s   | 39 ++++++++++++++++------------
 test/flags.sh | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+), 20 deletions(-)
 create mode 100755 test/flags.sh

diff --git a/.gitignore b/.gitignore
index 9c95afb..498cced 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,4 +5,5 @@
 /env
 /hexdump
 /prtest
+/flagtest
 *.o
diff --git a/flagtest.s b/flagtest.s
index f2a147a..5cab16e 100644
--- a/flagtest.s
+++ b/flagtest.s
@@ -9,9 +9,17 @@ flags:
 .dword 0 # value / default
 .byte 1 # type: <end>, bool, string, int
 .byte 5 # flag length
-#.short 16 # help length
-.ascii "-help"
-#.ascii "prints help text"
+.ascii "-bool"
+
+.dword 0
+.byte 2 # type=string
+.byte 4
+.ascii "-str"
+
+.dword 0
+.byte 3 # type=string
+.byte 4
+.ascii "-int"
 
 # last flag
 .dword 0
@@ -30,6 +38,8 @@ _start:
     la a0, flags
     call parse_flags
 
+
     li a7, 93 # sys_exit
-    li a0, 0
+    snez a0, a0
+    #li a0, 0
     ecall
diff --git a/lib/flags.s b/lib/flags.s
index aab0c17..5ecfe87 100644
--- a/lib/flags.s
+++ b/lib/flags.s
@@ -1,12 +1,9 @@
 
 .option norelax
+.option rvc
 .global init_flags
 .global parse_flags
 
-.macro subi dst, src, imm
-    addi \dst, \src, -\imm
-.endm
-
 .data
 
 flags:
@@ -29,9 +26,9 @@ saved_argv: .dword 0
 
 .section .rodata
 
-flag_failure_msg: .ascii "flag parsing failed\n\0"
+flag_failure_msg: .ascii "flag parsing failed\n"
 .equ flag_failure_msg_len, (.) - flag_failure_msg
-unknown_flag_msg: .ascii "unknown flag\n\0"
+unknown_flag_msg: .ascii "unknown flag\n"
 .equ unknown_flag_msg_len, (.) - unknown_flag_msg
 
 .text
@@ -75,6 +72,10 @@ parse_flags:
     tail parse_flags_args
     # n.b. tail clobbers t1
 
+.macro subi dst, src, imm
+    addi \dst, \src, -\imm
+.endm
+
 # a0 - flag definition struct
 # a1 - argc
 # a2 - argv
@@ -113,6 +114,7 @@ parse_flags_args:
 
 .Ldone:
     mv ra, a7
+    mv a0, x0
     ret
 
 .Lunknown_flag:
@@ -160,11 +162,15 @@ find_flag:
   1:# \0
     # is this a boolean flag? then there's no value
     beqz t3, handle_flag
-    # if not, set s1 to the next arg and advance s2
-    ld s1, 8(s2)
+    # if not, advance to the next arg
+    # a missing arg is an error
     addi s2, s2, 8
     subi a1, a1, 1
-    # TODO: if it's null, that's an error
+    beqz a1, .Lbadarg
+    # load arg into s1
+    # a null arg is an error
+    ld s1, 0(s2)
+    beqz s1, .Lbadarg
     # fallthrough
 
     # s1 now points to the value of the flag
@@ -189,7 +195,7 @@ boolflag:
     lbu t0, 0(s1)
     # if arg is empty, set value to true
     # TODO: should -bool and -bool= be treated differently?
-    beqz t0, 0f
+    beqz t0, 1f
     # check for "true" and "false"
     li t1, 'f'
     beq t0, t1, .Lboolcmpfalse
@@ -200,23 +206,23 @@ boolflag:
 
 .Lboolcmpfalse:
     lbu t0, 1(s1)
-    beqz t0, .Lbadbool
+    #beqz t0, .Lbadbool
     li t1, 'a'
     bne t1, t0, .Lbadbool
 
     lbu t0, 2(s1)
-    beqz t0, .Lbadbool
+    #beqz t0, .Lbadbool
     li t1, 'l'
     bne t1, t0, .Lbadbool
 
     lbu t0, 3(s1)
-    beqz t0, .Lbadbool
+    #beqz t0, .Lbadbool
     li t1, 's'
     bne t1, t0, .Lbadbool
 
     lbu t0, 4(s1)
-    beqz t0, .Lbadbool
-    li t1, 's'
+    #beqz t0, .Lbadbool
+    li t1, 'e'
     bne t1, t0, .Lbadbool
 
     lbu t0, 5(s1)
@@ -269,10 +275,11 @@ intflag:
     j .L_next_arg
 
 .Lbad_flag_def:
+.Lbadarg:
 .Lbadint:
 .Lbadbool:
     mv ra, a7
-    li a7, 64
+    li a7, 64 # sys_write
     li a0, 2
     la a1, flag_failure_msg
     li a2, flag_failure_msg_len
diff --git a/test/flags.sh b/test/flags.sh
new file mode 100755
index 0000000..8021222
--- /dev/null
+++ b/test/flags.sh
@@ -0,0 +1,71 @@
+#!/bin/bash
+set -eu
+set -o pipefail
+
+: ${BIN:=..}
+: ${EMU:=qemu-riscv64}
+
+cmd=$BIN/flagtest
+name=flagtest
+
+fail=0
+err() {
+    echo "FAIL $name: $*"
+    fail=1
+}
+
+flagtest() {
+    "$EMU" "$cmd" "$@"
+}
+
+check_fail() {
+    set +e
+    out=$(flagtest "$@" 2>&1)
+    if [[ "$?" -eq 0 ]]; then
+        err "expected command to fail: $cmd $*"
+    fi
+}
+
+check_ok() {
+    set +e
+    flagtest "$@"
+    if [[ "$?" -ne 0 ]]; then
+        err  "expected command to succeed: $cmd $*"
+    fi
+}
+
+# invalid
+check_fail -int      # missing arg
+check_fail -str
+check_fail -int x    # invalid int
+check_fail -int abc
+check_fail -int 0x10 # not yet supported
+check_fail -int ""   # empty string is not a valid number
+check_fail -int=
+check_fail -xxx      # invalid flag
+check_fail -bool=x   # invalid bool
+
+# valid
+check_ok
+check_ok -bool
+check_ok -bool=true
+check_ok -bool=false
+check_ok -int 0
+check_ok -int=1
+check_ok -int 987654321
+check_ok -str=x
+check_ok -str=
+check_ok -str ""
+check_ok -str -xxx # 2nd arg should be treated as a value not a flag
+check_ok -str x -str y # duplicates are allowed
+check_ok -int 1 -int 2
+check_ok -bool -bool
+check_ok X -bool Y # flags can be interspered with arguments
+check_ok -bool -str x -int 1
+
+# valid, but maybe it shouldn't be?
+check_ok -bool=
+
+# valid. and bool should not consume the "false"
+# (but we don't have a way to check that just yet)
+check_ok -bool false