Re: [Xen-devel] [PATCH v2] xenconsole: Add pipe option

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Felix Schmoll writes ("Re: [PATCH v2] xenconsole: Add pipe option"):
> As there is already an interactive variable in the code, it seems
> like a rather strange overloading to call the option interactive
> that directly affects a different variable (currently pipe). The
> name seems to make sense however,

Right, I think the UI should be driven by the needs of the human
who'll use it, not by the variable names in the code.

> so I propose to simplify the code
> by removing the isatty-check from line 349 and moving it to line
> 472, resulting in the following:
> 
> 472     if (isatty(STDIN_FILENO) && isatty(STDOUT_FILENO)) {
> 473         interactive = 1;
> 474         init_term(STDIN_FILENO, &stdin_old_attr);
> 475         atexit(restore_term_stdin); /* if this fails, oh dear */         
> 476     }
> 
> Then the interactive-variable is free for my purposes, so there is no need to
> introduce a new variable at all.
> 
> Or is there anything that requires the check to be at the top?

I doubt it.  Doing it after the option parsing loop would be much more
conventional.

> As the new commit message I suggest:
> 
> Add option to xenconsole to always forward console input
> 
> Currently the default behaviour of the xenconsole client is to
> ignore any input to stdin, unless stdin and stdout are both
> ttys. The new option allows to manually overwrite this, causing the
> client to forward input regardless.

SGTM.

Thanks,
Ian.

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