[SLOF] [PATCH v3 10/17] Add support for a TPM menu to control the state of the TPM

[Thomas Huth]

On 15.11.2017 17:43, Stefan Berger wrote:
> On 12/15/2015 07:34 AM, Thomas Huth wrote:
>> On 30/11/15 23:01, Stefan Berger wrote:
[...]
>>> diff --git a/board-qemu/slof/vtpm-sml.fs b/board-qemu/slof/vtpm-sml.fs
>>> index 193b567..3e17c82 100644
>>> --- a/board-qemu/slof/vtpm-sml.fs
>>> +++ b/board-qemu/slof/vtpm-sml.fs
>>> @@ -120,6 +120,200 @@ log-base LOG-SIZE tpm-set-log-parameters
>> ...
>>> +\ wait for keyboard input
>>> +: menu-key-get
>>> +    0 0 DO
>>> +       key? IF
>>> +           key
>>> +           UNLOOP EXIT
>>> +       THEN
>>> +       100 MS
>>> +    LOOP
>>> +    1b
>>> +;
>> Is that some kind of timeout that you've tried to do with the "100 MS"
>> and 1b at the end here? If so, that should have a proper comment. Then I
>> somewhat doubt that it is working as expected: Since you're looping with
>> "0 0" that means the loop is running almost forever - and since you're
>> delaying 256 ms inbetween (yes, we're hex), the loop will likely never
>> end while the user is sitting in front of the screen.
>> So if you really want to time-out here, I think you have to rework the
>> code. If you wanted to do an endless loop instead ... well then please
>> do not use "DO ... LOOP" and something like BEGIN ... WHILE ... REPEAT
>> or BEGIN ... UNTIL instead.
> 
> I may revive this patch series again... and re-learn Forth after the break.
> 
> Does this look ok? I assume 'key' blocks until it has keyboard input.
> 
>  \ wait for keyboard input
>  : menu-key-get
>     BEGIN key 1 UNTIL
>  ;

Well, that would work, but using the loop here is pretty much nonsense.
Let me translate that to C:

void menu_key_get(void)
{
	int ret;
	do {
		ret = getchar();
	while (0);
	return ret;
}

I hope you'll immediately say "just use getchar() directly instead"
here. Same with the Forth code: You likely don't need a new function
here, just use "key" directly to wait for a key press and get it's
keycode. Or did you want to have a function with a timeout instead?

 Thomas