cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: ftp enhancement - FTP wildcard download

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Thu, 28 Jan 2010 23:42:03 +0100 (CET)

On Wed, 27 Jan 2010, Pavel Raiskup wrote:

> First of all I'd like to ask you if there is a big problem with source size
> (binary size/number of source lines) or code complexity.

In general, no. But all added code adds to the amount of code we need to read
through to graps stuff so we should always strive to minimize it. The more
important detail is that the code should be written in a clear and concise way
with enough comments so that it becomes easy enough to read.

> I know, that it is possible improve library without recoursive and
> "ftp://site/*/*/*.c" idea, but it could be much more useable for app
> programmer..

Possibly, but with recursion you also get more features. Perhaps an app wants
different patterns in different subdirs? Perhaps the app wants to limit the
depth of the tree traversal? Perhaps the app wants to exclude a specific
single dir from the traversal?

If we can't offer these features, we may end up in a situation where all apps
that want recursion STILL implements it themselves simply because our way is
not flexible enough and then we have an implementation that adds complexity
for no additional benefit.

I'm not saying it has to become like this, I'm only saying we need to think
things through so that it doesn't.

> Next I really considering to parse LIST response. Even if we discard the
> recoursive idea (but I'd like rather do this), we have to let the app
> programmer know the matched filename is directory -- to allow him to create
> new curl handle and start with wildcard downloading in subdirectory.

Yes, I think you're right. It will make it less good if it treats everything
as files and thus fails the transfer on all entries that are actually
directories (or symlinks?) and not actual files.

I guess it will also make it a better function if it knows the total size of
all the files it will download, and the number of files so that the app will
know beforehand.

> And the multiple-calling curl_easy_perform idea -- it could be a little
> inconsistent behavior from libcurl and I think it is not really necessary.

Inconsistent, yes. But so is calling the write function with contents from
several files, so no matter which method we pick we're breaking new grounds
and break previous assumptions and paradigms at least slightly.

> int wildcard_callback(char *matchedstring, FILE *f, int filetype)

As Kamil Dudka brought up: FILE *f is really not the right type to pass to a
callback. It should be a 'void *' and the callback itself will then treat it
as whatever it knows was passed in.

I also miss total number of files and total size of all files (which you
mentioned further down in your mail but I didn't understand if you have
decided not to include them or not). What is 'matchedstring' ? Is that the
directory entry name?

I also like a differe error code usage. Just using TRUE/FALSE tends to end up
a mistake. Use 1 and 0 instead explictly, and everything else is reserved for
future use. That allows for a future libcurl to introduce new return codes
without breaking old apps.

> Question is where to close "FILE *f" when it is changing.

The wildcard callback is called before each transfer so it would basically say
"here comes a new file". That means the previous file ended and it would have
to be closed, or similar. A bit awkward I'd say (especially for the last
downloaded file). Possibly we should consider one callback before and one
after each file.

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Received on 2010-01-28