Re: pkg/43013: Clarification of URL handling for pkg_add/pkgin

[brook%biology.nmsu.edu@localhost (Brook Milligan)]

Joerg,

I applied your patch, but the same problem remains.  As a result, I've
been looking over the code some more and have a few points to note.

I think I have narrowed down the problem that I am seeing.  Here is
one section of the calling sequence as indicated by some printf()
statements I added (note that the URL I gave to pkg_add was
ftp://localhost/All/emacs and that the file All/emacs-23.1nb2.tgz
exists in the ftp login directory):

     fetchList(): pattern=emacs*, flags=c
     fetchListFTP(): NLST
     ftp_request(): op=NLST, op_arg=emacs*, flags=c
     ftp_request(): path=/All
     ftp_cwd(): path=/All, subdir=1
     ftp_cwd(): path=All
     ftp_cwd(): conn->ftp_home=(null)
     ftp_cwd(): strchr(path, '/')=(null)

I think the key point here is the subdir argument to ftp_cwd, which is
initialized by the expression 'op_arg != NULL' in ftp_request().
Since I am requesting /All/emacs* that value seems correct to indicate
that the path requested for cwd (i.e., /All) is a subdirectory not a
file.  However, here is the code that is executed within ftp_cwd
immediately after the printf() above:

        /* Simple case: still in the home directory and no directory change. */
        if (conn->ftp_home == NULL && strchr(path, '/') == NULL)
                return 0;

Clearly, this condition holds and ftp_cwd immediately exits.

My analysis of this suggests, however, that only if subdir=0 should
this exit immediately.  When there is not a '/' in the path, there are
two cases: (i) the path points to a file (i.e., subdir=0) or (ii) the
path points to the directory in which the file resides (i.e.,
subdir=1).  In the example I have above, the second case holds and one
CWD command, not zero, should be emitted.

If all this logic is correct, I would expect something like the
following instead:

--- ftp.c
+++ ftp.c.orig
@@ -304,7 +304,7 @@
        ++path;
 
        /* Simple case: still in the home directory and no directory change. */
-       if (conn->ftp_home == NULL && strchr(path, '/') == NULL)
+       if (conn->ftp_home == NULL && strchr(path, '/') == NULL && !subdir)
                return 0;
 
        if ((e = ftp_cmd(conn, "PWD\r\n")) != FTP_WORKING_DIRECTORY ||

Indeed, that change makes this example complete correctly.

Since you are much more familiar with all of this code than I, please
verify that my logic is correct and that there are not other
implications of the change I suggest.

Thanks alot for taking a look at this.

Cheers,
Brook