Re: [PATCH] Extending XL console command to take -e option.

[Wei Liu <wei.liu2@citrix.com>]

The title should reference "xen console client" instead of "XL console".

On Thu, Mar 31, 2016 at 12:46:11AM +0530, sabiya wrote:
> By default, Character 0x1d i.e '^]' is used as escape character to terminate logging.
> Now,escape character can be passed with -escape option.

Doesn't long option require double dashes? It should be "--escape" I
think.

> It can be any printable character.
> As valid range for control character is first 32 characters(0-31d)
> lower 5 bits of given character will be used as escape character.
> 
> Example: xl console -escape a testDomain

xl console --escape a testDomain

> 
> Signed-off-by: Sabiya Kazi <sabiyafk@gmail.com>
> ---
>  tools/console/client/main.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/console/client/main.c b/tools/console/client/main.c
> index d006fdc..6e303f5 100644
> --- a/tools/console/client/main.c
> +++ b/tools/console/client/main.c
> @@ -35,6 +35,7 @@
>  #include <sys/select.h>
>  #include <err.h>
>  #include <string.h>
> +#include <ctype.h>
>  #ifdef __sun__
>  #include <sys/stropts.h>
>  #endif
> @@ -42,13 +43,12 @@
>  #include <xenstore.h>
>  #include "xenctrl.h"
>  #include "_paths.h"
> -

Stray blank line change. Please don't include such change.

>  #define ESCAPE_CHARACTER 0x1d
>  
>  static volatile sig_atomic_t received_signal = 0;
>  static char lockfile[sizeof (XEN_LOCK_DIR "/xenconsole.") + 8] = { 0 };
>  static int lockfd = -1;
> -
> +static char escapechar = ESCAPE_CHARACTER;

There should be a blank line here.

>  static void sighandler(int signum)
>  {
>  	received_signal = 1;
> @@ -214,7 +214,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] == escapechar) {
>  				return 0;
>  			} 
>  
> @@ -318,6 +318,14 @@ static void console_unlock(void)
>  	}
>  }
>  
> +/*
> + *In ASCII, Valid range for control characters is 0-31
> + *Lower 5 bits of given char will be used as escape character.
> + */
> +char getEscapeChar(const char *s) {
> +    return (*s ^ 0x40);
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	struct termios attr;
> @@ -329,6 +337,7 @@ int main(int argc, char **argv)
>  	struct option lopt[] = {
>  		{ "type",     1, 0, 't' },
>  		{ "num",     1, 0, 'n' },
> +		{ "escape",     1, 0, 'n' },
                                       ^ 'e'

>  		{ "help",    0, 0, 'h' },
>  		{ 0 },
>  
> @@ -363,6 +372,11 @@ int main(int argc, char **argv)
>  				exit(EINVAL);
>  			}
>  			break;
> +		case 'e' :
> +		    escapechar = getEscapeChar(optarg);
> +            break;

Indentation is wrong.

Final question, did you test this patch in action?

Wei.

> +
> +
>  		default:
>  			fprintf(stderr, "Invalid argument\n");
>  			fprintf(stderr, "Try `%s --help' for more information.\n", 
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel