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

[Heinrich Schuchardt]

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.

Best regards

Heinrich

> 
> 
> Alex
> 
> 
>