Bad OS_Byte 129 use in the wimp

Hiya,

This is a simple bug in the Wimp `pollforkey` which does not follow the documented behaviour.

OS_Byte 129 (INKEY) is explicitly documented as having the following exit conditions :

If reading a key within a time limit:

    R1 = ASCII code if character read, else undefined
    R2 = &00 if character read, &1B if an escape condition exists, or &FF if timeout

The Wimp does this (https://gitlab.riscosopen.org/RiscOS/Sources/Desktop/Wimp/-/blob/master/s/Wimp01#L5948):

        MOV     R0,#&81                 ; read key with 0 time limit
        MOV     R1,#0
        MOV     R2,#0
        SWI     XOS_Byte
;
        TEQ     R1,#&FF
        Debug   err,"Key ?"
        Pull    "R2,R3,PC",LS           ; EQ or CC

So there are two behaviours that are incorrect here.

1. It’s checking R1, which is explicitly declared to be undefined. It should be checking R2.
2. It’s relying on the state of the carry flag on return from the call, which is not defined at all in the documentation.

The actual behaviour of the Kernel is to set C if no key was seen, and clear C if a key was seen, but that’s not documented. It does appear to be documented for the BBC (see https://beebwiki.mdfs.net/OSBYTE_%2681)

 On entry: Y<128 - XY=centisecond timeout - INKEY(0) to INKEY(32767)
 On exit:  Cy=0, Y=0,   X=character : character returned
           Cy=1, Y=255, X preserved : timed out
           Cy=1, Y=27   X preserved : Escape key pressed
 The caller can test for either Cy=0 or Y=0 for a valid returned character.
 If Cy=1 or Y<>0 the contents of X should not be relied on.

and my copy of the BBC Advanced user guide states:

On exit,
   If a character is detected, X=ASCII value of key pressed, Y=0 and C=0.
   If a character is not detected within timeout then Y=&FF and C=1 If Escape is pressed then Y=&1B (27) and C=1.

So whilst BBC wiki has it explicitly documented that X (R1 in RISC OS) should not be relied on, the AUG says nothing (and presumably should not be relied on).

Therefore, the Wimp is relying on BBC behaviour (and undocumented RISC OS behaviour) for the status of the C flag, and an implementation detail for the state of R1.

The Wimp can be changed to comply with documented behaviour by something like:

    TEQ     r2, #27
    BEQ     %FT20
    TEQ     r2, #255
    Pull    "R2,R3,PC"         ; Z=0 if key pressed; Z=1 if no key
20
; acknowledge the escape condition

Do you think the kernel behaviour in relation to the C flag ought to be documented as well, seeing as what has been inherited from the BBC has probably been that way throughout the existence of RISC OS? Or is it best left undocumented, but unchanged in case other applications rely on it.

Nasty – looks suspiciously like someone changed a CMP to TEQ (and the register is wrong).

Do you think the kernel behaviour in relation to the C flag ought to be documented as well, seeing as what has been inherited from the BBC has probably been that way throughout the existence of RISC OS?

It definitely ought to be documented as it’s one of the reasons _kernel_swi_c was concocted.

Does anyone have a copy of the RISC OS 2 PRMs – was it in those? I did once have a set as part-payment for proofreading them but think they disappeared at Colton Software sometime.

[Or even the RO2 WM source? Somewhat unlikely. I think most of us avoided TEQ in favour of CMP unless there was a really good reason to TEQ (e.g. TEQVCP).]

Nasty – looks suspiciously like someone changed a CMP to TEQ (and the register is wrong).

Yeah I thought that, but then noticed that the register was wrong, too.

Does anyone have a copy of the RISC OS 2 PRMs – was it in those?

Additional reference:

Archimedes manual: https://www.4corn.co.uk/archive/docs/Archimedes%20Programmer’s%20Reference%20Manual%20-%20Volume%201-opt.pdf

Page 140, PDF page 147 – no documentation of the state of C flag. However R1 is defined to be 255 if there was a timeout, so there’s some lineage here for that.

RISCOS 2 manual: https://www.4corn.co.uk/archive/docs/RISC%20OS%20Programmer’s%20Reference%20Manual%20-%20Volume%202-opt.pdf

Page 498, PDF page 43 – R1 declared as undefined, and no documentation of the C flag.

Do you think the kernel behaviour in relation to the C flag ought to be documented as well

Both fixing the Wimp and documenting the behaviour should be done.

Without documentation it’s not possible to know what the behaviour should be, and the intent is more important if you’re trying to keep things working in the future and in past versions.

For example a claimant of ByteV is entirely at liberty to corrupt R1 in the case where escape or no key is pressed, according to the documentation. If they do that, things won’t work. That’s not their fault – that’s the documentation and the Wimp’s fault for not doing what was documented.

When implementing Pyromaniac the interfaces are being implemented using the documentation whereever possible (except in those cases where I’m sufficiently confident that I can do it from memory – which is fallible!). Hence why this failure was found. The implementation of the ‘read with timeout’ was:

    # Read with timeout
    timeout = (high * 256) + low
    key = ro.kernel.input.getchar(timeout=timeout/100.0)
    if key is Escape:
        regs[1] = 0
        regs[2] = 0x1B
    elif key is None:
        regs[1] = 0
        regs[2] = 0xFF
    else:
        regs[1] = ord(key)
        regs[2] = 0

It has now been updated to report the C flag in order to keep the Wimp happy (and I’ve fixed the Wimp as suggested above).

This is the great thing about having your independent implementation!

I may be a twit, but can’t access those URLs.

Does anyone have a copy of the RISC OS 2 PRMs – was it in those? I did once have a set as part-payment for proofreading them but think they disappeared at Colton Software sometime.

On the RO2 PRM (page 498):

R0 – 129 (&81)

Read keyboard for information

…

On Exit

    R1 = ASCII code if character read else undefined

    R2 = &00 if character read

         &1B if an escape conditions exists

         &FF if timeout

So identical to what Gerph reported and just for the sake of clarity also Arthur Programmer reference reports the same.

Arthur PRM page 140:

On exit:

    R1 = ASCII code, or &FF if R1 = &FF (timed out)

    R2 = &00 if character read , &1B if escape pressed, &FF if timeout

Hope this helps

oh Sorry, Gerph already posted the info while I was seeking for them

This is the great thing about having your independent implementation!

There’s limited behaviour that has surprised me, although I’m working with modules from Select and I had already been stripping out the uses of undefined behaviour and private interfaces, so I may be working from a better base.

There was something recently that surprised me, although it wasn’t undefined behaviour – there was some module that was using the StartupFS interface triggered by the OS_Byte version of the service call in order to determine whether a filing system was present. I think that was it.

ResourceFS Filer! That was it! I fixed that in my copy but it’s nuts. My version just checks whether Resources:$ exists and if so continues. I’ll create a separate topic for that as it’s come up.

I may be a twit, but can’t access those URLs.

I think the links have been mangled by either copying or the forum software – certainly they’re not being linked for the entire length of the URL. I got them from here: https://www.4corn.co.uk/articles/docs/ – so you should be able to find them some way down the page. I can’t remember if an apostrophe is meant to be escaped or not, but it’s just what came out of Firefox :-(

Ta.

I think the links have been mangled by either copying or the forum software

A pet hate: Spaces in filenames – the gift that keeps kicking you in the nads.

The spaces aren’t the problem in this case.

The spaces aren’t the problem in this case

They started the mangling sequence by causing the html links to be using %20 and thus difficult to read.
Then you get on to the bit that the link visibly breaks: Programmer’s

That errant apostrophe…

Arthur PRM page 140:
On exit:
R1 = ASCII code, or &FF if R1 = &FF (timed out)
R2 = &00 if character read , &1B if escape pressed, &FF if timeout

Is this correct?

Is this correct?

Depends how literal you want to be.

Clearly R1 will be &FF if R1 is &FF. ☺

However, given the mention of the timeout, I think it’s reasonable to suggest that it’s a typo and they meant R1 will be &FF if R2 is &FF.

That being said, I would like to draw your attention to the fifth paragraph on page 355 (that’s the very start of book two).

However, it is clear that any major piece of software written for the A-series machines should be implemented as a module, so this chapter is included as a guide for the writer of such software.

There is so much wrong with that statement that a part of me wonders if it got slipped in as a dare to see if anybody noticed.
At any rate, I wouldn’t use the Arthur PRM as the authoritative source of anything beyond Arthur itself.

I don’t remember anyone giving the Arthur PRM the once-over! We did proofread RISC OS 2 PRMs.