On Fri, Jul 16, 2010 at 10:22:54AM +0200, Peter Kümmel wrote:
> Enrico Forestieri wrote:
>
> >> -SystemcallPrivate::SystemcallPrivate(const std::string& of) :
> >> +SystemcallPrivate::SystemcallPrivate(const std::string& of) :
> >> proc_(new QProcess), outindex_(0),
> >> errindex_(0),
> >> - outfile(of), showout_(false),
> >> showerr_(false), process_events(false)
> >> + outfile(of),
> >> +
> >> terminalOutExists_(os::is_terminal(os::STDOUT)),
> >> +
> >> terminalErrExists_(os::is_terminal(os::STDERR)),
> >> + process_events(false)
> >> {
> >> if (!outfile.empty()) {
> >> // Check whether we have to simply throw away the output.
> >> if (outfile != os::nulldev())
> >> proc_->setStandardOutputFile(toqstr(outfile));
> >> - } else if (os::is_terminal(os::STDOUT))
> >> - setShowOut(true);
> >
> > This is wrong. You are going to output to a terminal whenever one exists.
> > That was not the logic of what you changed. Output to a terminal should
> > be avoided when there isn't a terminal *or* when stdout is redirected
> > to a file. Imagine you have "pnmtopng foo.pnm > foo.png", which produces
> > binary output. With your change the terminal will be flooded by binary
> > data, possibly putting it in a strange state, because in the binary stream
> > something interpretable as an escape sequence may be present.
> >
>
> I never panned to understand this redirecting ;)
>
> So, is this patch correct:
>
> Index: Systemcall.cpp
> ===================================================================
> --- Systemcall.cpp (Revision 34914)
> +++ Systemcall.cpp (Arbeitskopie)
> @@ -246,7 +246,7 @@
> out_index_(0),
> err_index_(0),
> out_file_(of),
> -
> terminal_out_exists_(os::is_terminal(os::STDOUT)),
> + terminal_out_exists_(false),
>
> terminal_err_exists_(os::is_terminal(os::STDERR)),
> process_events_(false)
> {
> @@ -254,6 +254,8 @@
> // Check whether we have to simply throw away the output.
> if (out_file_ != os::nulldev())
> process_->setStandardOutputFile(toqstr(out_file_));
> + } else if (os::is_terminal(os::STDOUT)) {
> + terminal_out_exists_ = true;
> }
The following would be more concise:
@@ -254,6 +254,8 @@
// Check whether we have to simply throw away the output.
if (out_file_ != os::nulldev())
process_->setStandardOutputFile(toqstr(out_file_));
+ // Don't output to terminal if stdout is redirected
+ terminal_out_exists_ = false;
}
Then, are you sure that sending binary data also to the progress
interface doesn't cause any harm?
--
Enrico
