Re: [PATCH v3 1/2] tools/console: Add escape argument to configure escape character

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Jun 22, 2023 at 03:12:47PM +0100, Peter Hoyes wrote:
> From: Peter Hoyes <Peter.Hoyes@xxxxxxx>
> 
> Dom0 may be accessed via telnet, meaning the default escape character
> (which is the same as telnet's) cannot be directly used to exit the
> console. It would be helpful to make the escape character customizable
> in such use cases.
> 
> Add --escape argument to console tool for this purpose.
> 
> Create parse_escape_character static function to convert a character
> string (which may include a '^' modifier) into an ANSI integer.
> 
> Add argument to getopt options, parse escape character and pass value
> to console_loop.
> 
> If --escape is not specified, it falls back to the existing behavior
> using DEFAULT_ESCAPE_SEQUENCE.
> 
> Signed-off-by: Peter Hoyes <Peter.Hoyes@xxxxxxx>
> Reviewed-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
>  tools/console/client/main.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/console/client/main.c b/tools/console/client/main.c
> index 6775006488..fb7cfb04b5 100644
> --- a/tools/console/client/main.c
> +++ b/tools/console/client/main.c
> @@ -42,7 +42,7 @@
>  #include <xenstore.h>
>  #include "xenctrl.h"
>  
> -#define ESCAPE_CHARACTER 0x1d
> +#define DEFAULT_ESCAPE_CHARACTER 0x1d
>  
>  static volatile sig_atomic_t received_signal = 0;
>  static char lockfile[sizeof (XEN_LOCK_DIR "/xenconsole.") + 8] = { 0 };
> @@ -77,6 +77,7 @@ static void usage(const char *program) {
>              "  -n, --num N      use console number N\n"
>              "  --type TYPE      console type. must be 'pv', 'serial' or 
> 'vuart'\n"
>              "  --start-notify-fd N file descriptor used to notify parent\n"
> +            "  --escape E       escape sequence to exit console\n"
>              , program);
>  }
>  
> @@ -174,7 +175,7 @@ static void restore_term(int fd, struct termios *old)
>  }
>  
>  static int console_loop(int fd, struct xs_handle *xs, char *pty_path,
> -                     bool interactive)
> +                     bool interactive, char escape_character)
>  {
>       int ret, xs_fd = xs_fileno(xs), max_fd = -1;
>  
> @@ -215,7 +216,7 @@ static int console_loop(int fd, struct xs_handle *xs, 
> char *pty_path,
>                       char msg[60];
>  
>                       len = read(STDIN_FILENO, msg, sizeof(msg));
> -                     if (len == 1 && msg[0] == ESCAPE_CHARACTER) {
> +                     if (len == 1 && msg[0] == escape_character) {
>                               return 0;
>                       } 
>  
> @@ -335,6 +336,7 @@ int main(int argc, char **argv)
>               { "help",    0, 0, 'h' },
>               { "start-notify-fd", 1, 0, 's' },
>               { "interactive", 0, 0, 'i' },
> +             { "escape",  1, 0, 'e' },
>               { 0 },
>  
>       };
> @@ -345,6 +347,7 @@ int main(int argc, char **argv)
>       console_type type = CONSOLE_INVAL;
>       bool interactive = 0;
>       const char *console_names = "serial, pv, vuart";
> +     char escape_character = DEFAULT_ESCAPE_CHARACTER;
>  
>       while((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
>               switch(ch) {
> @@ -375,6 +378,12 @@ int main(int argc, char **argv)
>               case 'i':
>                       interactive = 1;
>                       break;
> +             case 'e':
> +                     if (optarg[0] == '^')
> +                             escape_character = optarg[1] & 0x1f;
> +                     else
> +                             escape_character = optarg[0];
> +                     break;

Could you check that `optarg` actually contains a valid escape
character sequence? Either only 1 character, or '^' followed by only one
char.

(If one want to use the NUL char to escape, it should set ^@, not '' or
'^'.)

Thanks,

-- 
Anthony PERARD