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

[Thomas Singer]

Hi Frank,

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

Very strange, for me CVS 1.12.9 seemed to have solved the problem. But I 
have no problem giving 2 free Professional licenses away, one to the one who 
half-fixed it in CVS 1.12.9 and one to you for the final solution ;)

--
Best regards,
Thomas Singer
_____________
smartcvs.com



Frank Hemer schrieb:

> 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
>
>
>
> _______________________________________________
> Bug-cvs mailing list
> Bug-cvs@gnu.org
> http://lists.gnu.org/mailman/listinfo/bug-cvs