On Tue, 18 Jan 2011, Mele Giovanni wrote:

Thanks for your contribution!

> Here are my changes. I used version 7.21.3 of curl to start from.

Please use "diff -u" to make diffs. It worked fine this time, but if you don't
use -u there's a much higher risk that we can't apply the patch in our ends!

> I'm sorry but I'm new in this kinf of work in a dev community. What is
> top-posting???

Top-posting is that mailing style where you quote the full mail, and you
insert your own comments on-top above the quote. It forces everyone to read
the mail backwards (from bottom to top) to follow the thread, and it also
encourages full-quoting and that's also a bad idea.

There are over one thousand recipiants of each mail sent to this list. I think
we can demand that everyone who sends mails here makes an effort to make their
mails easy-to-read and focused on the point they want to express. (ie that
top-posting is quick or easy or "default" or whatever is not a valid reason
for making a thousand persons spend extra time in order to digest the mail).
http://daniel.haxx.se/blog/2009/09/20/why-top-posting-annoys-me/

Now, about your patch:

- if(data->set.opt_no_body)
+ /* Do we want file info? */
+ if((data->set.opt_no_body) &&
+ (sftp_scp->path[strlen(sftp_scp->path)-1] == '/'))
            state(conn, SSH_STOP);

This logic is a bit flawed. data->set.opt_no_body is the CURLOPT_NOBODY which
the apps sets if it doesn't want a "body" to get transfered. If you don't
stop the state machine there when that is set true, you must make sure to set
it later...

+ /* Test if we must just send back file info if no body option is set
*/
+ if((data->set.opt_no_body) &&

This condition is flawed. An app should be able to extract the information
even if it decides to transfer the body. The condition should rather check if
data->set.get_filetime is set, as that is a request to get that information.
But since the operation is basically free there, as the info is already
around, I think we can do it unconditionally. The download size as well.

But as I mentioned above, when the info has been retrieved and set.opt_no_body
is true, the state machine must be stopped with state(conn, SSH_STOP);

+ /* Test if this is a folder or a file */
+ if (attrs.permissions & 0x8000) { /* this is a file */

I think I prefer using the macros/defines that libssh2 provides for this.
Like:

    if( LIBSSH2_SFTP_S_ISREG(attrs.permissions)) {

Since this macro was introduced in libssh2 1.2.5 and we might want to still
compile with older versions we can provide our own alternative at the top of
the ssh.c file in case it isn't already defined!

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html