| Message ID | 20180406084704.29048-2-lvivier@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix output word | expand |
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>
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
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
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
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
On 8/4/18 9:05 pm, 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. Laurent, you are reposting this patch with a fix, right? thanks. > 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 > _______________________________________________ > SLOF mailing list > SLOF@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/slof >
On 14/05/2018 11:55, Alexey Kardashevskiy wrote: > On 8/4/18 9:05 pm, 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. > > > Laurent, you are reposting this patch with a fix, right? thanks. Alexey, I've sent a v7 last week. It has been reviewed by Thomas. Regarding the comments from Segher, I will not change the code because review is clearer like that (not indentation changes), but if you commit this series could you update the comment as he asks ("xt" instead of "xt-handle")? Thanks, Laurent
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
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(-)