cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: PATCH: Establishing data connections in FTP has been made non-blocking for multi interface

From: Gokhan Sengun <gokhansengun_at_gmail.com>
Date: Thu, 22 Dec 2011 16:17:43 +0200

Hi there,

Thanks you too for the encouragement and help afterwards in cleaning up the
> way things are done. I am going to apply your changes some torture a few
> days later to see everything is still working but after a quick look, I
> think I noticed a potential bug.
>
>
I have not had enough time to apply your follow-up fixes a torture test yet
but I have found a few issues that needs to be considered:

1- There is a typo in function Curl_do_more(), variable "completed" needs
to be dereferenced, currently the if statement is always true.

  if(!result && completed)
    /* do_complete must be called after the protocol-specific DO function */
    do_complete(conn);

2- Code is broken when establishing the data connection is not immediately
possible but after a while possible. I think it is because, function
ftp_do_more() returns after accepting the server connect however it needs
to fall through and set "*complete" to TRUE before exit from the function.

    if(ftpc->wait_data_conn == TRUE) {
      bool serv_conned;

      result = ReceivedServerConnect(conn, &serv_conned);
      if(result)
        return result; /* Failed to accept data connection */

      if(serv_conned) {
        /* It looks data connection is established */
        result = AcceptServerConnect(conn);
        ftpc->wait_data_conn = FALSE;
        if(result == CURLE_OK)
          result = InitiateTransfer(conn);
      }

      return result; // ===========> GSENG this needs to be removed
    }

I think it will be a good idea to simulate
not-immediately-available-but-after-a-while-available-data-connection-establishment-case
with a test. I do not have much experience with the test system to simulate
this.

Thanks.

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