[U-Boot] [PATCH 1/2] efi_loader: cursor positioning

[Alexander Graf]

> Am 05.09.2019 um 22:35 schrieb Heinrich Schuchardt <xypron.glpk at gmx.de>:
> 
>> On 9/5/19 10:21 PM, Alexander Graf wrote:
>>> On 05.09.19 10:06, Heinrich Schuchardt wrote:
>>> When backspacing in column 0 do no set the column index to ULONG_MAX.
>>> Ensure that the row number is not set to ULONG_MAX even if the row count is
>>> advertised as 0.
>>> Ignore control characters other the 0x08, 0x0a, 0x0d when updating the
>>> column.
>>> 
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>>   lib/efi_loader/efi_console.c | 12 ++++++++----
>>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>> index d4765afb98..d5222c46b4 100644
>>> --- a/lib/efi_loader/efi_console.c
>>> +++ b/lib/efi_loader/efi_console.c
>>> @@ -156,13 +156,14 @@ static efi_status_t EFIAPI efi_cout_output_string(
>>>        * Update the cursor position.
>>>        *
>>>        * The UEFI spec provides advance rules for U+0000, U+0008, U+000A,
>>> -     * and U000D. All other characters, including control characters
>>> -     * U+0007 (BEL) and U+0009 (TAB), have to increase the column by one.
>>> +     * and U000D. All other control characters are ignored. Any non-control
>>> +     * character increase the column by one.
>>>        */
>>>       for (p = string; *p; ++p) {
>>>           switch (*p) {
>>>           case '\b':    /* U+0008, backspace */
>>> -            con->cursor_column = max(0, con->cursor_column - 1);
>>> +            if (con->cursor_column)
>>> +                con->cursor_column--;
>>>               break;
>>>           case '\n':    /* U+000A, newline */
>>>               con->cursor_column = 0;
>>> @@ -178,13 +179,16 @@ static efi_status_t EFIAPI efi_cout_output_string(
>>>                */
>>>               break;
>>>           default:
>>> -            con->cursor_column++;
>>> +            if (*p > 0x1f)
>> What is the 0x1f here? I know, control character, but it's not obvious. Probably wants either a comment or a define.
> 
> I will add a comment
> 
>>> +                con->cursor_column++;
>>>               break;
>>>           }
>>>           if (con->cursor_column >= mode->columns) {
>>>               con->cursor_column = 0;
>>>               con->cursor_row++;
>>>           }
>>> +        if (con->cursor_row >= mode->rows && con->cursor_row)
>>> +            con->cursor_row--;
>> I don't understand this statement. When is the cursor_row >= mode->rows? Is this just to offset an incorrect cursor_row from the line above? Can't we just fold it in there?
> 
> The row is incremented either by line feed or by exceeding the last column.
> 
> When we exceed the row limit the terminal will take care of scrolling. We have to compensate for the scrolling by decrementing the row.

Please indicate either through a variable or a comment that this compensates for already executed scrolling.


Alex