On 7 July 2015 at 22:07, Roy Franz <roy.fr...@linaro.org> wrote:
> On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel
> <ard.biesheu...@linaro.org> wrote:
>> On 7 July 2015 at 01:04, Roy Franz <roy.fr...@linaro.org> wrote:
>>> Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This
>>> is
>>> sent by many Linux terminals by default. Also accept VT220 function keys
>>> F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
>>> The VT220 escape codes are longer, and variable length so a new state is
>>> added
>>> to the state machine along with a variable to construct the multibyte escape
>>> sequence.
>>> There are currently no ambiguous escape sequence prefixes accepted, so the
>>> TTY
>>> terminal accepts escape sequences for a variety of terminals. The goal is
>>> to
>>> 'just work' with as many terminals as possible, rather than properly
>>> emulating
>>> any specific terminal. Backspace, Del, and F10 have been tested on xterm,
>>> rxvt, tmux, and screen.
>>> Note: The existing vt100 function key handling does not match the vt100
>>> documentation that I found, so I added the TTY terminal handling
>>> of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
>>> has no F5-F10 keys, so I don't know what the current vt100 code
>>> is based on.
>>>
>>> Signed-off-by: Roy Franz <roy.fr...@linaro.org>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>
>> Same here ^^^
>> (and in patch 1/4)
>
> I'll fix this here and in the other patches. Maybe I should re-read
> my contributions.txt patch so I get this right :)
:-)
>>
>>> ---
>>> MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
>>> MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95
>>> +++++++++++++++++++-
>>> 2 files changed, 95 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>>> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>>> index 03542a4..4616ab3 100644
>>> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>>> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>>> @@ -118,6 +118,7 @@ typedef struct {
>>> #define INPUT_STATE_LEFTOPENBRACKET 0x04
>>> #define INPUT_STATE_O 0x08
>>> #define INPUT_STATE_2 0x10
>>> +#define INPUT_STATE_LEFTOPENBRACKET_2 0x20
>>>
>>> #define RESET_STATE_DEFAULT 0x00
>>> #define RESET_STATE_ESC_R 0x01
>>> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
>>> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
>>> index 227df85..12e7f9f 100644
>>> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
>>> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
>>> @@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
>>> UINT16 UnicodeChar;
>>> EFI_INPUT_KEY Key;
>>> BOOLEAN SetDefaultResetState;
>>> + static UINT16 TtyEscapeStr[3];
>>> + static INTN TtyEscapeIndex;
>>>
>>
>> Is this guaranteed to be safe?
> I'm assuming you mean the length of 3? (for 2 characters + NUL termination)
> I think it is.
>
No, I mean the static. I know UEFI is single threaded, but it has an
event model that could potentially result in functions being called in
a reentrant fashion. I think this can only happen in functions that
signal events themselves, but I am not intimate enough with this stuff
to claim that this is 100% safe.
> I do the following when the INPUT_STATE_LEFTOPENBRACKET_2 is entered:
> + TtyEscapeStr[0] = UnicodeChar;
> + TtyEscapeIndex = 1;
>
> When any additional escape sequence characters are processed any values
> of TtyEscapeIndex other than 1 or 2 are errors. If we got an escape sequence
> that was too long, we will go back to the RESET_STATE_DEFAULT state,
> and TtyEscape* will be re-initialized when the INPUT_STATE_LEFTOPENBRACKET_2
> state is entered.
>
> There is only 1 place where INPUT_STATE_LEFTOPENBRACKET_2 is entered,
> and both TtyEscape* variables are initialized there, and only valid
> (ie Index of 1 or 2)
> index values are processed once in INPUT_STATE_LEFTOPENBRACKET_2.
>
> I can add some comments to this effect if you think it is worthwhile.
>
>
>>
>>> TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);
>>>
>>> @@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
>>> continue;
>>> }
>>>
>>> - if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE)
>>> {
>>> + if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE
>>> ||
>>> + TerminalDevice->TerminalType ==
>>> TTYTERMTYPE)) {
>>> TerminalDevice->InputState |= INPUT_STATE_O;
>>> TerminalDevice->ResetState = RESET_STATE_DEFAULT;
>>> continue;
>>> @@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
>>> default :
>>> break;
>>> }
>>> + } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
>>> + /* Also accept VT100 escape codes for F1-F4 for TTY term */
>>> + switch (UnicodeChar) {
>>> + case 'P':
>>> + Key.ScanCode = SCAN_F1;
>>> + break;
>>> + case 'Q':
>>> + Key.ScanCode = SCAN_F2;
>>> + break;
>>> + case 'R':
>>> + Key.ScanCode = SCAN_F3;
>>> + break;
>>> + case 'S':
>>> + Key.ScanCode = SCAN_F4;
>>> + break;
>>> + }
>>> }
>>>
>>> if (Key.ScanCode != SCAN_NULL) {
>>> @@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
>>> }
>>> }
>>>
>>> + /*
>>> + * The VT220 escape codes that the TTY terminal accepts all have
>>> + * numeric codes, and there are no ambiguous prefixes shared with
>>> + * other terminal types.
>>> + */
>>> + if (TerminalDevice->TerminalType == TTYTERMTYPE &&
>>> + Key.ScanCode == SCAN_NULL &&
>>> + UnicodeChar >= '0' &&
>>> + UnicodeChar <= '9') {
>>> + TtyEscapeStr[0] = UnicodeChar;
>>> + TtyEscapeIndex = 1;
>>> + TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
>>> + continue;
>>> + }
>>> +
>>> if (Key.ScanCode != SCAN_NULL) {
>>> Key.UnicodeChar = 0;
>>> EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
>>> @@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
>>> break;
>>>
>>>
>>> + case INPUT_STATE_ESC | INPUT_STATE_LEFTOPENBRACKET |
>>> INPUT_STATE_LEFTOPENBRACKET_2:
>>> + /*
>>> + * Here we handle the VT220 escape codes that we accept. This
>>> + * state is only used by the TTY terminal type.
>>> + */
>>> + Key.ScanCode = SCAN_NULL;
>>> + if (TerminalDevice->TerminalType == TTYTERMTYPE) {
>>> +
>>> + if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
>>> + UINTN EscCode;
>>> + TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
>>> + EscCode = StrDecimalToUintn(TtyEscapeStr);
>>> + switch (EscCode) {
>>> + case 3:
>>> + Key.ScanCode = SCAN_DELETE;
>>> + break;
>>> + case 11:
>>> + case 12:
>>> + case 13:
>>> + case 14:
>>> + case 15:
>>> + Key.ScanCode = SCAN_F1 + EscCode - 11;
>>> + break;
>>> + case 17:
>>> + case 18:
>>> + case 19:
>>> + case 20:
>>> + case 21:
>>> + Key.ScanCode = SCAN_F6 + EscCode - 17;
>>> + break;
>>> + case 23:
>>> + case 24:
>>> + Key.ScanCode = SCAN_F11 + EscCode - 23;
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> + } else if (TtyEscapeIndex == 1){
>>> + /* 2 character escape code */
>>> + TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
>>> + continue;
>>> + }
>>> + else {
>>> + DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
>>> + }
>>> + }
>>> + TerminalDevice->ResetState = RESET_STATE_DEFAULT;
>>> +
>>> + if (Key.ScanCode != SCAN_NULL) {
>>> + Key.UnicodeChar = 0;
>>> + EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
>>> + TerminalDevice->InputState = INPUT_STATE_DEFAULT;
>>> + UnicodeToEfiKeyFlushState (TerminalDevice);
>>> + continue;
>>> + }
>>> +
>>> + UnicodeToEfiKeyFlushState (TerminalDevice);
>>> + break;
>>> +
>>> default:
>>> //
>>> // Invalid state. This should never happen.
>>
>> Changing state machine code like this makes me nervous, but as far as
>> I can tell, non-TtyTerm users are not affected by it, so there is
>> little risk of regression.
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
