[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: redirecting stderr failure on :ext: ssh and large data
From: |
Frank Hemer |
Subject: |
Re: redirecting stderr failure on :ext: ssh and large data |
Date: |
Thu, 12 Aug 2004 21:25:18 +0200 |
User-agent: |
KMail/1.5.1 |
In addition to my prev. mail I would not claim the patch is 'correct' in terms
of the final solution. Still it is a simple workaround for a problem that
hasn't been fixed for years.
If cvs doesn't take the possibility into account that its stderr might be
modified by a subprocess causing data loss under certain circumstances
without even dropping a warning makes me think it's a bug anyway.
Would it make sense to pipe the subprocess stderr output to cvs and forward it
to stderr in a non-blocking aware save way?
Frank
> > I just discovered a bug with stderr redirection.
>
> Which version of cvs did you test? We were recently informed that this
> problem may have been fixed in cvs 1.12.9 (although that is by no means
> certain).
>
> > In case some app starts cvs as coprocess and has redirected stderr
> > before:
> >
> > dup2(STDOUT_FILENO,STDERR_FILENO);
> >
> > cvs will lose data. This only applies to :ext: method with CVS_RSH set to
> > ssh and a filesize of a few thousand lines (tested with diff
> > --side-by-side).
> >
> > Changing the following code in client.c/handle_m(args, len) will fix
this:
> > >>>>>>>>>>>>>>>>> old >>>>>>>>>>>>>>>>>>
> >
> > fflush (stderr);
> > fwrite (args, len, sizeof (*args), stdout);
> > putc ('\n', stdout);
> >
> > <<<<<<<<<<<<<<<< new <<<<<<<<<<<<<<<<<<
> >
> > fflush (stderr);
> >
> > fd_set wfds;
> > FD_ZERO(&wfds);
> > FD_SET(STDOUT_FILENO, &wfds);
> > int s = select(STDOUT_FILENO+1,NULL,&wfds,NULL,NULL);
> > if (s < 1) {
> > perror("cannot write to stdout");
> > }
> >
> > fwrite (args, len, sizeof (*args), stdout);
> > putc ('\n', stdout);
> >
> > >>>>>>>>>>>>>>> end >>>>>>>>>>>>>>>>>>>
>
> Would a similar change need to be made to handle_e () as well?
>
> See this thread for all the gory details of the previous time this
> problem was raised:
>
> <http://lists.gnu.org/archive/html/bug-cvs/2002-07/msg00084.html>
>
> as well as the recent thread on bug-cvs
>
> <http://lists.gnu.org/archive/html/bug-cvs/2004-08/msg00024.html>
>
> which also discusses the problem.
>
> In light of the above threads, do you really believe that your patch is
> correct?
>
> -- Mark