Hi all,

On Tue, 22 May 2012, Gokhan Sengun wrote:

> > Out of curiosity do you know what the knock on effect of this
> > is, by not calling smtp_dophase_done() ?
> >
> > I see from the code that it is already a little ambiguous as to
> > whether smtp_dophase_done() or smtp_disconnect() frees
> > up the domain for example.
>
> I can only confirm there is no memory leak occuring.
>
> > Would it not be better to do:
>
[snip]
>
> it looks to a better idea, I copied FTP's way without big considerations.

No problem - it was good to see that this solved the issue. I've had a look at the ftp code as well, and whilst I appreciate I don't know the ftp code too well, it seems a little worrying for ftp_dophase_done() to not be called if result is an error code representing a problem with the transfer, etc... especially as part of that function is supposed to close the second socket if an error occurred :-/

As such I would like to propose this fix for smtp:

static CURLcode smtp_doing(struct connectdata *conn, bool *dophase_done)
{
  CURLcode result = smtp_multi_statemach(conn, dophase_done);

  if(result)
    DEBUGF(infof(conn->data, "DO phase failed\n"));
  else
    DEBUGF(infof(conn->data, "DO phase is complete\n"));

  if(*dophase_done)
    smtp_dophase_done(conn, FALSE /* not connected */);

  return result;
}

You will note it is a slight reworking of what I proposed early as it may be useful to have the debug information in there ;-)

Are you able to give this a quick test in your environment and all being well, if Daniel et al don't have any issues with this I will push it a little later this evening (UK time).

Kind Regards

Steve

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2012-05-22