> Now for my questions:

- Line 2359 in lib/ftp.c now has a comment that says:
> /* TODO: gseng - do it once for connected */
>
> Can you explain that for us? Do what and why is it a TODO and not done
> now?
>
>
It is left there mistakenly. I remember I added some TODOs to simulate "not
immediately established" data connections with real FTP servers because
most of the servers send SYN request before "150 -- " therefore data
connection is immediately available to be established.

> - You only did a very minor change to lib/multi.c but I don't like it:
>
> case CURLM_STATE_DO_DONE:
> +
> + if(easy->easy_conn->bits.wait_**data_conn == TRUE) {
> + multistate(easy, CURLM_STATE_DO_MORE);
> + result = CURLM_OK;
> + break;
> + }
> +
>
> Why would you switch to the CURLM_STATE_DO_DONE state in the first place
> if
> the conditions say it should actually be in the CURLM_STATE_DO_MORE
> state?
> I don't like this. The condition should be added to the code that changed
> the state in the first place I think, unless you have a proper motivation
> for this peculiarity.
>

Touching multi.c file was the easiest way for me to take the control. In
initial implementation, I added the check after

case CURLM_STATE_DO_MORE:

and see if wait_data_conn bit is set, if so I just set the result to
CURLM_OK and returned then expected to continue from CURLM_STATE_DO_MORE
state. This worked almost OK except running the executable with valgrind or
strace. I introduced a few more info statements and saw that wait_data_conn
bit only set while multi interface is in CURLM_STATE_DO_DONE state not in
CURLM_STATE_DO_MORE. I took the easiest path and implemented as the
delivered patch.

I would be really appreciate if you could see how could this be enhanced. I
am also not very proud of the change I have done in multi.c file :-)

Thanks,

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