[v6,1/2] Fix output word

Message ID 20180406084704.29048-2-lvivier@redhat.com
State New
Headers show
Series
  • Fix output word
Related show

Commit Message

Laurent Vivier April 6, 2018, 8:47 a.m.
We can select the console output, but it does not really work

Implement term-io-emit, as we have term-io-key to really
send characters to the output selected by stdout.

Resolve xt-handle and ihandle in the output command.

Use them in the new term-io-emit function.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 slof/fs/term-io.fs | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

Comments

Thomas Huth April 6, 2018, 9:03 a.m. | #1
On 06.04.2018 10:47, Laurent Vivier wrote:
> We can select the console output, but it does not really work
> 
> Implement term-io-emit, as we have term-io-key to really
> send characters to the output selected by stdout.
> 
> Resolve xt-handle and ihandle in the output command.
> 
> Use them in the new term-io-emit function.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  slof/fs/term-io.fs | 36 +++++++++++++++++++++++++++++++-----
>  1 file changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs
> index 52ce12a..d247b09 100644
> --- a/slof/fs/term-io.fs
> +++ b/slof/fs/term-io.fs
> @@ -10,6 +10,17 @@
>  \ *     IBM Corporation - initial implementation
>  \ ****************************************************************************/
>  
> +0 VALUE write-xt
> +
> +VARIABLE stdout
> +
> +: set-stdout ( ihandle -- )
> +   \ Close old stdout:
> +   stdout @ ?dup IF close-dev THEN
> +   \ Now set the new stdout:
> +   dup stdout !
> +   encode-int s" stdout" set-chosen
> +;
>  
>  : input  ( dev-str dev-len -- )
>     open-dev ?dup IF
> @@ -24,12 +35,15 @@
>  
>  : output  ( dev-str dev-len -- )
>     open-dev ?dup IF
> -      \ Close old stdout:
> -      s" stdout" get-chosen IF
> -         decode-int nip nip ?dup IF close-dev THEN
> +      \ find new ihandle and xt handle
> +      dup s" write" rot ihandle>phandle find-method
> +      0= IF
> +         drop
> +         cr ." Cannot find the write method for the given output console " cr
> +         EXIT
>        THEN
> -      \ Now set the new stdout:
> -      encode-int s" stdout" set-chosen
> +      to write-xt
> +      set-stdout
>     THEN
>  ;
>  
> @@ -40,6 +54,18 @@
>  
>  1 BUFFER: (term-io-char-buf)
>  
> +: term-io-emit ( char -- )
> +    write-xt IF
> +       (term-io-char-buf) c!
> +       (term-io-char-buf) 1 write-xt stdout @ call-package
> +       drop
> +    ELSE
> +       [ ' emit behavior compile, ]
> +    THEN
> +;
> +
> +' term-io-emit to emit
> +
>  : term-io-key  ( -- char )
>     s" stdin" get-chosen IF
>        decode-int nip nip dup 0= IF 0 EXIT THEN
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>
Segher Boessenkool April 8, 2018, 11:05 a.m. | #2
Hi!

On Fri, Apr 06, 2018 at 10:47:03AM +0200, Laurent Vivier wrote:
>  : output  ( dev-str dev-len -- )
>     open-dev ?dup IF

It's a lot simple if you do an early exit if there is no such device.
You're supposed to output an error message in that case, fwiw.

> -      \ Close old stdout:
> -      s" stdout" get-chosen IF
> -         decode-int nip nip ?dup IF close-dev THEN
> +      \ find new ihandle and xt handle
> +      dup s" write" rot ihandle>phandle find-method
> +      0= IF
> +         drop
> +         cr ." Cannot find the write method for the given output console " cr
> +         EXIT

This leaks the instance.  The description in the standard say to search
for the method before doing the open-dev, that solves this problem too.

> +: term-io-emit ( char -- )
> +    write-xt IF
> +       (term-io-char-buf) c!
> +       (term-io-char-buf) 1 write-xt stdout @ call-package
> +       drop
> +    ELSE
> +       [ ' emit behavior compile, ]
> +    THEN
> +;

What do you expect to be in the emit hook at this point?  Could you
just use _that_?

Looks nice and readable otherwise, great :-)


Segher
Thomas Huth April 9, 2018, 6:15 a.m. | #3
On 08.04.2018 13:05, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Apr 06, 2018 at 10:47:03AM +0200, Laurent Vivier wrote:
>>  : output  ( dev-str dev-len -- )
>>     open-dev ?dup IF
> 
> It's a lot simple if you do an early exit if there is no such device.
> You're supposed to output an error message in that case, fwiw.
> 
>> -      \ Close old stdout:
>> -      s" stdout" get-chosen IF
>> -         decode-int nip nip ?dup IF close-dev THEN
>> +      \ find new ihandle and xt handle
>> +      dup s" write" rot ihandle>phandle find-method
>> +      0= IF
>> +         drop
>> +         cr ." Cannot find the write method for the given output console " cr
>> +         EXIT
> 
> This leaks the instance.  The description in the standard say to search
> for the method before doing the open-dev, that solves this problem too.
> 
>> +: term-io-emit ( char -- )
>> +    write-xt IF
>> +       (term-io-char-buf) c!
>> +       (term-io-char-buf) 1 write-xt stdout @ call-package
>> +       drop
>> +    ELSE
>> +       [ ' emit behavior compile, ]
>> +    THEN
>> +;
> 
> What do you expect to be in the emit hook at this point?  Could you
> just use _that_?

The code in slof/fs/term-io.fs is "common" code, i.e. shared between all
"boards", so there is no 100% generic way to say which token should be
expected in the emit hook at this point in time. That's why we have the
detour with [ ' ... behavior compile, ] here. Unless we want to
hard-code a default name for all boards (ok, we currently only have two
boards, but still...), I think that line is a nice and short solution to
the problem.

 Thomas
Segher Boessenkool April 9, 2018, 11:21 a.m. | #4
On Mon, Apr 09, 2018 at 08:15:37AM +0200, Thomas Huth wrote:
> On 08.04.2018 13:05, Segher Boessenkool wrote:
> >> +    ELSE
> >> +       [ ' emit behavior compile, ]
> >> +    THEN
> >> +;
> > 
> > What do you expect to be in the emit hook at this point?  Could you
> > just use _that_?
> 
> The code in slof/fs/term-io.fs is "common" code, i.e. shared between all
> "boards", so there is no 100% generic way to say which token should be
> expected in the emit hook at this point in time. That's why we have the
> detour with [ ' ... behavior compile, ] here. Unless we want to
> hard-code a default name for all boards (ok, we currently only have two
> boards, but still...), I think that line is a nice and short solution to
> the problem.

The usual solution is to just have two hooks, and have one call the
other.  So

DEFER debug-emit

...
  ELSE debug-emit THEN ;

(or call it early-emit, or whatever makes sense).


Segher
Thomas Huth April 9, 2018, 11:54 a.m. | #5
On 09.04.2018 13:21, Segher Boessenkool wrote:
> On Mon, Apr 09, 2018 at 08:15:37AM +0200, Thomas Huth wrote:
>> On 08.04.2018 13:05, Segher Boessenkool wrote:
>>>> +    ELSE
>>>> +       [ ' emit behavior compile, ]
>>>> +    THEN
>>>> +;
>>>
>>> What do you expect to be in the emit hook at this point?  Could you
>>> just use _that_?
>>
>> The code in slof/fs/term-io.fs is "common" code, i.e. shared between all
>> "boards", so there is no 100% generic way to say which token should be
>> expected in the emit hook at this point in time. That's why we have the
>> detour with [ ' ... behavior compile, ] here. Unless we want to
>> hard-code a default name for all boards (ok, we currently only have two
>> boards, but still...), I think that line is a nice and short solution to
>> the problem.
> 
> The usual solution is to just have two hooks, and have one call the
> other.  So
> 
> DEFER debug-emit
> 
> ...
>   ELSE debug-emit THEN ;
> 
> (or call it early-emit, or whatever makes sense).

Looks like we've already got serial-key, serial-key? and serial-emit on
both board-js2x and board-qemu, so I assume Laurent could simply use
these here...

 Thomas

Patch

diff --git a/slof/fs/term-io.fs b/slof/fs/term-io.fs
index 52ce12a..d247b09 100644
--- a/slof/fs/term-io.fs
+++ b/slof/fs/term-io.fs
@@ -10,6 +10,17 @@ 
 \ *     IBM Corporation - initial implementation
 \ ****************************************************************************/
 
+0 VALUE write-xt
+
+VARIABLE stdout
+
+: set-stdout ( ihandle -- )
+   \ Close old stdout:
+   stdout @ ?dup IF close-dev THEN
+   \ Now set the new stdout:
+   dup stdout !
+   encode-int s" stdout" set-chosen
+;
 
 : input  ( dev-str dev-len -- )
    open-dev ?dup IF
@@ -24,12 +35,15 @@ 
 
 : output  ( dev-str dev-len -- )
    open-dev ?dup IF
-      \ Close old stdout:
-      s" stdout" get-chosen IF
-         decode-int nip nip ?dup IF close-dev THEN
+      \ find new ihandle and xt handle
+      dup s" write" rot ihandle>phandle find-method
+      0= IF
+         drop
+         cr ." Cannot find the write method for the given output console " cr
+         EXIT
       THEN
-      \ Now set the new stdout:
-      encode-int s" stdout" set-chosen
+      to write-xt
+      set-stdout
    THEN
 ;
 
@@ -40,6 +54,18 @@ 
 
 1 BUFFER: (term-io-char-buf)
 
+: term-io-emit ( char -- )
+    write-xt IF
+       (term-io-char-buf) c!
+       (term-io-char-buf) 1 write-xt stdout @ call-package
+       drop
+    ELSE
+       [ ' emit behavior compile, ]
+    THEN
+;
+
+' term-io-emit to emit
+
 : term-io-key  ( -- char )
    s" stdin" get-chosen IF
       decode-int nip nip dup 0= IF 0 EXIT THEN