grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCHv2 2/3] core: use GRUB_TERM_ definitions when handling term ch


From: Pete Batard
Subject: Re: [PATCHv2 2/3] core: use GRUB_TERM_ definitions when handling term characters
Date: Mon, 7 Aug 2017 17:21:56 +0100
User-agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

This is the updated proposal from the original "[PATCH 2/3] crypto: switch nonstandard escape sequence to its hex value".

Following up on Vladimir's comments, I decided to look up all the GRUB sources where key/term entries were being compared to '\e', and replaced those with GRUB_TERM_ESC, with GRUB_TERM_ESC changed to use a hex value.

To be consistent, since we also have definitions for GRUB_TERM_BACKSPACE and GRUB_TERM_TAB in term.h, this patch also replaces '\b' and '\t' where appropriate (i.e. mostly for keypress validation, but not for printed output). Obviously the result is that quite a few more files are affected compared to the original patch.

As per this proposal, the only files that are still being left with '\e' sequences are argp-fmtstream.c in gnulib, since I'm not too sure of its use and thought it better to leave it alone, as well as terminfo.c which also contains a series of Escape sequences (e.g. "\e[H\e[J") that I'm not too concerned about.

The last thing I'll point out is that, because term.h is included as part of normal.h, there wasn't any need to add new #include directives for the GRUB_TERM_ defs. Of course, this patch was also validated to confirm it doesn't break compilation.

Regards,

/Pete
From 231792eeb40bf6ef590ba2dfc9615f8ab725e779 Mon Sep 17 00:00:00 2001
From: Pete Batard <address@hidden>
Date: Mon, 7 Aug 2017 16:20:30 +0100
Subject: [PATCH 2/3] core: use GRUB_TERM_ definitions when handling term
 characters

* Also use hex value for GRUB_TERM_ESC as '\e' is not in the C standard and is 
not understood by some compilers
---
 grub-core/commands/keylayouts.c    |  6 +++---
 grub-core/commands/menuentry.c     |  4 ++--
 grub-core/lib/crypto.c             |  4 ++--
 grub-core/normal/auth.c            |  6 +++---
 grub-core/normal/cmdline.c         |  4 ++--
 grub-core/normal/menu.c            |  2 +-
 grub-core/normal/menu_entry.c      |  2 +-
 grub-core/normal/menu_text.c       |  4 ++--
 grub-core/term/efi/console.c       |  2 +-
 grub-core/term/i386/pc/console.c   |  2 +-
 grub-core/term/terminfo.c          | 14 +++++++-------
 grub-core/tests/cmdline_cat_test.c |  2 +-
 grub-core/tests/gfxterm_menu.c     |  2 +-
 include/grub/term.h                |  3 ++-
 14 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/grub-core/commands/keylayouts.c b/grub-core/commands/keylayouts.c
index f4b7730..f35d3a3 100644
--- a/grub-core/commands/keylayouts.c
+++ b/grub-core/commands/keylayouts.c
@@ -40,7 +40,7 @@ static struct grub_keyboard_layout layout_us = {
     /* 0x10 */ 'm',  'n',  'o',  'p',  'q', 'r', 's', 't',
     /* 0x18 */ 'u',  'v',  'w',  'x',  'y', 'z', '1', '2',
     /* 0x20 */ '3',  '4',  '5',  '6',  '7', '8', '9', '0',
-    /* 0x28 */ '\n', '\e', '\b', '\t', ' ', '-', '=', '[',
+    /* 0x28 */ '\n', GRUB_TERM_ESC, GRUB_TERM_BACKSPACE, GRUB_TERM_TAB, ' ', 
'-', '=', '[',
     /* According to usage table 0x31 should be mapped to '/'
        but testing with real keyboard shows that 0x32 is remapped to '/'.
        Map 0x31 to 0. 
@@ -82,8 +82,8 @@ static struct grub_keyboard_layout layout_us = {
     /* 0x10 */ 'M',  'N',  'O',  'P',  'Q', 'R', 'S', 'T',
     /* 0x18 */ 'U',  'V',  'W',  'X',  'Y', 'Z', '!', '@',
     /* 0x20 */ '#',  '$',  '%',  '^',  '&', '*', '(', ')',
-    /* 0x28 */ '\n' | GRUB_TERM_SHIFT, '\e' | GRUB_TERM_SHIFT, 
-    /* 0x2a */ '\b' | GRUB_TERM_SHIFT, '\t' | GRUB_TERM_SHIFT, 
+    /* 0x28 */ '\n' | GRUB_TERM_SHIFT, GRUB_TERM_ESC | GRUB_TERM_SHIFT,
+    /* 0x2a */ GRUB_TERM_BACKSPACE | GRUB_TERM_SHIFT, GRUB_TERM_TAB | 
GRUB_TERM_SHIFT,
     /* 0x2c */ ' '  | GRUB_TERM_SHIFT,  '_', '+', '{',
     /* According to usage table 0x31 should be mapped to '/'
        but testing with real keyboard shows that 0x32 is remapped to '/'.
diff --git a/grub-core/commands/menuentry.c b/grub-core/commands/menuentry.c
index 58d4dad..2c5363d 100644
--- a/grub-core/commands/menuentry.c
+++ b/grub-core/commands/menuentry.c
@@ -52,8 +52,8 @@ static struct
   int key;
 } hotkey_aliases[] =
   {
-    {"backspace", '\b'},
-    {"tab", '\t'},
+    {"backspace", GRUB_TERM_BACKSPACE},
+    {"tab", GRUB_TERM_TAB},
     {"delete", GRUB_TERM_KEY_DC},
     {"insert", GRUB_TERM_KEY_INSERT},
     {"f1", GRUB_TERM_KEY_F1},
diff --git a/grub-core/lib/crypto.c b/grub-core/lib/crypto.c
index 683a8aa..ca334d5 100644
--- a/grub-core/lib/crypto.c
+++ b/grub-core/lib/crypto.c
@@ -462,7 +462,7 @@ grub_password_get (char buf[], unsigned buf_size)
       if (key == '\n' || key == '\r')
        break;
 
-      if (key == '\e')
+      if (key == GRUB_TERM_ESC)
        {
          cur_len = 0;
          break;
@@ -487,7 +487,7 @@ grub_password_get (char buf[], unsigned buf_size)
   grub_xputs ("\n");
   grub_refresh ();
 
-  return (key != '\e');
+  return (key != GRUB_TERM_ESC);
 }
 #endif
 
diff --git a/grub-core/normal/auth.c b/grub-core/normal/auth.c
index 7338f82..6be678c 100644
--- a/grub-core/normal/auth.c
+++ b/grub-core/normal/auth.c
@@ -166,13 +166,13 @@ grub_username_get (char buf[], unsigned buf_size)
       if (key == '\n' || key == '\r')
        break;
 
-      if (key == '\e')
+      if (key == GRUB_TERM_ESC)
        {
          cur_len = 0;
          break;
        }
 
-      if (key == '\b')
+      if (key == GRUB_TERM_BACKSPACE)
        {
          if (cur_len)
            {
@@ -197,7 +197,7 @@ grub_username_get (char buf[], unsigned buf_size)
   grub_xputs ("\n");
   grub_refresh ();
 
-  return (key != '\e');
+  return (key != GRUB_TERM_ESC);
 }
 
 grub_err_t
diff --git a/grub-core/normal/cmdline.c b/grub-core/normal/cmdline.c
index a36180d..c037d50 100644
--- a/grub-core/normal/cmdline.c
+++ b/grub-core/normal/cmdline.c
@@ -626,12 +626,12 @@ grub_cmdline_get (const char *prompt_translated)
            cl_insert (cl_terms, nterms, &lpos, &llen, &max_len, &buf, 
kill_buf);
          break;
 
-       case '\e':
+       case GRUB_TERM_ESC:
          grub_free (cl_terms);
          grub_free (buf);
          return 0;
 
-       case '\b':
+       case GRUB_TERM_BACKSPACE:
          if (lpos > 0)
            {
              lpos--;
diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
index 719e2fb..e7a83c2 100644
--- a/grub-core/normal/menu.c
+++ b/grub-core/normal/menu.c
@@ -763,7 +763,7 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot)
               *auto_boot = 0;
              return current_entry;
 
-           case '\e':
+           case GRUB_TERM_ESC:
              if (nested)
                {
                  menu_fini ();
diff --git a/grub-core/normal/menu_entry.c b/grub-core/normal/menu_entry.c
index eeeee55..cdf3590 100644
--- a/grub-core/normal/menu_entry.c
+++ b/grub-core/normal/menu_entry.c
@@ -1403,7 +1403,7 @@ grub_menu_entry_run (grub_menu_entry_t entry)
            goto fail;
          break;
 
-       case '\e':
+       case GRUB_TERM_ESC:
          destroy_screen (screen);
          return;
 
diff --git a/grub-core/normal/menu_text.c b/grub-core/normal/menu_text.c
index e22bb91..16b6c16 100644
--- a/grub-core/normal/menu_text.c
+++ b/grub-core/normal/menu_text.c
@@ -237,8 +237,8 @@ print_entry (int y, int highlight, grub_menu_entry_t entry,
       data->geo.first_entry_x, y });
 
   for (i = 0; i < len; i++)
-    if (unicode_title[i] == '\n' || unicode_title[i] == '\b'
-       || unicode_title[i] == '\r' || unicode_title[i] == '\e')
+    if (unicode_title[i] == '\n' || unicode_title[i] == GRUB_TERM_BACKSPACE
+       || unicode_title[i] == '\r' || unicode_title[i] == GRUB_TERM_ESC)
       unicode_title[i] = ' ';
 
   if (data->geo.num_entries > 1)
diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index 7d31095..02f64ea 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -104,7 +104,7 @@ const unsigned efi_codes[] =
     GRUB_TERM_KEY_DC, GRUB_TERM_KEY_PPAGE, GRUB_TERM_KEY_NPAGE, 
GRUB_TERM_KEY_F1,
     GRUB_TERM_KEY_F2, GRUB_TERM_KEY_F3, GRUB_TERM_KEY_F4, GRUB_TERM_KEY_F5,
     GRUB_TERM_KEY_F6, GRUB_TERM_KEY_F7, GRUB_TERM_KEY_F8, GRUB_TERM_KEY_F9,
-    GRUB_TERM_KEY_F10, GRUB_TERM_KEY_F11, GRUB_TERM_KEY_F12, '\e'
+    GRUB_TERM_KEY_F10, GRUB_TERM_KEY_F11, GRUB_TERM_KEY_F12, GRUB_TERM_ESC
   };
 
 static int
diff --git a/grub-core/term/i386/pc/console.c b/grub-core/term/i386/pc/console.c
index 28de46b..f6142a2 100644
--- a/grub-core/term/i386/pc/console.c
+++ b/grub-core/term/i386/pc/console.c
@@ -204,7 +204,7 @@ static int
 grub_console_getkey (struct grub_term_input *term __attribute__ ((unused)))
 {
   const grub_uint16_t bypass_table[] = {
-    0x0100 | '\e', 0x0f00 | '\t', 0x0e00 | '\b', 0x1c00 | '\r', 0x1c00 | '\n'
+    0x0100 | GRUB_TERM_ESC, 0x0f00 | GRUB_TERM_TAB, 0x0e00 | 
GRUB_TERM_BACKSPACE, 0x1c00 | '\r', 0x1c00 | '\n'
   };
   struct grub_bios_int_registers regs;
   unsigned i;
diff --git a/grub-core/term/terminfo.c b/grub-core/term/terminfo.c
index f0d3e3d..7e45026 100644
--- a/grub-core/term/terminfo.c
+++ b/grub-core/term/terminfo.c
@@ -257,7 +257,7 @@ grub_terminfo_gotoxy (struct grub_term_output *term,
   else
     {
       if ((pos.y == data->pos.y) && (pos.x == data->pos.x - 1))
-       data->put (term, '\b');
+       data->put (term, GRUB_TERM_BACKSPACE);
     }
 
   data->pos = pos;
@@ -357,7 +357,7 @@ grub_terminfo_putchar (struct grub_term_output *term,
     case '\a':
       break;
 
-    case '\b':
+    case GRUB_TERM_BACKSPACE:
     case 127:
       if (data->pos.x > 0)
        data->pos.x--;
@@ -426,12 +426,12 @@ grub_terminfo_readkey (struct grub_term_input *term, int 
*keys, int *len,
     }
   *len = 1;
   keys[0] = c;
-  if (c != ANSI_CSI && c != '\e')
+  if (c != ANSI_CSI && c != GRUB_TERM_ESC)
     {
       /* Backspace: Ctrl-h.  */
       if (c == 0x7f)
-       c = '\b'; 
-      if (c < 0x20 && c != '\t' && c!= '\b' && c != '\n' && c != '\r')
+       c = GRUB_TERM_BACKSPACE;
+      if (c < 0x20 && c != GRUB_TERM_TAB && c!= GRUB_TERM_BACKSPACE && c != 
'\n' && c != '\r')
        c = GRUB_TERM_CTRL | (c - 1 + 'a');
       *len = 1;
       keys[0] = c;
@@ -487,7 +487,7 @@ grub_terminfo_readkey (struct grub_term_input *term, int 
*keys, int *len,
          GRUB_TERM_KEY_HOME, GRUB_TERM_KEY_END };
     unsigned i;
 
-    if (c == '\e')
+    if (c == GRUB_TERM_ESC)
       {
        CONTINUE_READ;
 
@@ -606,7 +606,7 @@ grub_terminfo_getkey (struct grub_term_input *termi)
                         &data->npending, data->readkey);
 
 #if defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275)
-  if (data->npending == 1 && data->input_buf[0] == '\e'
+  if (data->npending == 1 && data->input_buf[0] == GRUB_TERM_ESC
       && grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_BROKEN_REPEAT)
       && grub_get_time_ms () - data->last_key_time < 1000
       && (data->last_key & GRUB_TERM_EXTENDED))
diff --git a/grub-core/tests/cmdline_cat_test.c 
b/grub-core/tests/cmdline_cat_test.c
index f1e2143..baea768 100644
--- a/grub-core/tests/cmdline_cat_test.c
+++ b/grub-core/tests/cmdline_cat_test.c
@@ -103,7 +103,7 @@ cmdline_cat_test (void)
                                             '/', 't', 'e', 's', 't', '.',
                                             't', 'x', 't', '\n',
                                             GRUB_TERM_NO_KEY,
-                                            GRUB_TERM_NO_KEY, '\e'},
+                                            GRUB_TERM_NO_KEY, GRUB_TERM_ESC},
                                         23);
 
       grub_video_checksum ("cmdline_cat");
diff --git a/grub-core/tests/gfxterm_menu.c b/grub-core/tests/gfxterm_menu.c
index 8f63dc2..12836fb 100644
--- a/grub-core/tests/gfxterm_menu.c
+++ b/grub-core/tests/gfxterm_menu.c
@@ -146,7 +146,7 @@ gfxterm_menu (void)
            return;
          }
        grub_terminal_input_fake_sequence ((int []) { -1, -1, -1, 
GRUB_TERM_KEY_DOWN, -1, 'e',
-             -1, GRUB_TERM_KEY_RIGHT, -1, 'x', -1,  '\e', -1, '\e' }, 14);
+             -1, GRUB_TERM_KEY_RIGHT, -1, 'x', -1,  GRUB_TERM_ESC, -1, 
GRUB_TERM_ESC }, 14);
 
        grub_video_checksum (tests[j].name);
 
diff --git a/include/grub/term.h b/include/grub/term.h
index 5ffb38f..8117e2a 100644
--- a/include/grub/term.h
+++ b/include/grub/term.h
@@ -55,7 +55,8 @@
 #define GRUB_TERM_KEY_INSERT    (GRUB_TERM_EXTENDED | 0x52)
 #define GRUB_TERM_KEY_CENTER    (GRUB_TERM_EXTENDED | 0x4c)
 
-#define GRUB_TERM_ESC          '\e'
+/* Hex value is used for ESC, since '\e' is nonstandard */
+#define GRUB_TERM_ESC          0x1b
 #define GRUB_TERM_TAB          '\t'
 #define GRUB_TERM_BACKSPACE    '\b'
 
-- 
2.9.3.windows.2


reply via email to

[Prev in Thread] Current Thread [Next in Thread]