Re: redirecting stderr failure on :ext: ssh and large data

[Frank Hemer]

I tested this with cvs 1.11.17. It is reproducible on several systems - only 
linux with kernel 2.4.* and 2.6.* -, win seems to be ok.

Reading the mentioned threads doesn't make me think the patch wouldn't work, 
in fact it works for my testing.
The result of fwrite is not checked, and sometimes returns 'device temporarily 
not available' which causes the data to be lost. This can not happen with 
seelect because the blocking of select is not affected by fd's being in 
nonblocking mode. So handle_m will simply block in case it cannot write the 
data. This shouldn't have any impact on performance.

From looking at the code it appears that the same should apply to handle_e, 
the difference is the amount of data to be written. So maybe this problem 
will never occur on stderr?
Maybe with some testcase and enough data it might happen too? At least some 
error output would help tracking this down. 

The bug is not fixed in cvs1.12.9 release, I run my tests on it and the prev. 
problems appear.

Frank

> Frank Hemer <frank@hemer.org> writes:
> > 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