bug-cvs
[Top][All Lists]
Advanced

[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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]