On Thursday 28 March 2002 20:14, Koos Vriezen wrote:
> On Thu, 28 Mar 2002, David Faure wrote:
> 
> > The patch looks good on the whole (thanks for the testing you did on it!)
> >
> > The SIGPIPE stuff looks good too. The first patch which wanted to remove the exit
> > in SlaveBase::sigpipe_handler was wrong because that handler is also called when
> > the application exits (the sigpipe comes from the app pipe in that case). So this
> > approach is better since it allows to distinguish between the two cases for sigpipes...
> > well, assuming the app doesn't crash/exit at the exact moment of this write() :}
> >
> > The one thing that surprises me is:
> > -    if ( cmd=="list" && maxretries > 0 ) // Only retry for "list". retr/stor/... need to redo the whole thing
> > +    if ( maxretries > 0 ) // Only retry for "list". retr/stor/... need to redo the whole thing  {
> > Hmm, I'm sure I added that comment for a reason...
> > Yes, one needs to do the PASV stuff first, etc.
> > Without the test, isn't this going to try and do the RETR or STOR
> > before the initial setup stuff like PASV, EPSV, etc. ?
> > (or if I'm wrong, then the comment must be removed ;)
> 
> I removed it because a timeout with "cwd" wouldn't reconnect. Didn't look
> that carefully at retr/stor, but probably always calling
> ftpSendCmd("xxx", 0) in ftpOpenCommand should be enough.

Sounds good.

No idea how to support timeouts with stor/retr though (can't be an idle
timeout anyway, more like the ftp server being shutdown I guess).

-- 
David FAURE, david@mandrakesoft.com, faure@kde.org
http://people.mandrakesoft.com/~david/, http://www.konqueror.org/
KDE, Making The Future of Computing Available Today