On Tue, Oct 17, 2006 at 06:31:33PM -0400, James Housley wrote:
> I have taken most of your feed back from the first patch set.

A few more comments:

- The CURLOPT identifiers, which form a part of the curl API, should not
include the specific SSH implementation being used by libcurl. e.g.
instead of CURLOPT_LIBSSH2_AUTH_TYPES call it CURLOPT_SSH_AUTH_TYPES,
so that it stays valid if someone adds support for an alternative SSH
implementation in the future (such as has happened for SSL).

- Code has been added to auto-detect sftp and scp transfers by looking at
host names beginning with sftp and scp. Are there really hosts out there
like scp.example.com or sftp.example.net? If so, then the checks should
really include a dot (matching 'scp.') so that someone trying to download
the home page of the Center for Surveillance, Control and Prevention of
Hand Injuries (curl scphi.com) gets an HTTP download instead of an SCP
one. (I think the similiar check for "ftps" instead of "ftps." should be
changed, too; the Fuller Termite & Pest Solutions, Inc. people might start to
complain.)

- There are a few places where HAVE_LIBSSH2 is checked, but it probably
ought to be USE_LIBSSH2

- The code includes the comment "This implementation ignores the host
name in conformance with RFC 1738. Only local files..." which looks to me
like a cut-and-paste problem. There are also several other references
to file: URLs that should be removed.

- Does libssh2 really support pkg-config? The packages I've seen don't
include pkg-config files.

- CURLVERSION_FORTH should be CURLVERSION_FOURTH

> curl -O scp://housley@baby:~/rc.conf <-- works

Shouldn't this URL be scp://housley@baby/~/rc.conf ?

>>> Dan

-- 
http://www.MoveAnnouncer.com              The web change of address service
          Let webmasters know that your web site has moved