On Wednesday 30 of December 2009 00:13:35 Björn Stenberg wrote:
> Here's a patch adding a new option: --remote-header-name. This option is a
> qualifier to -O that uses the filename in a Content-disposition: header
> line if it exists.

Looks good, though I haven't tested it yet.

> @@ -3310,7 +3320,10 @@
> /* open file for writing */
> out->stream=fopen(out->filename, "wb");
> if(!out->stream) {
> - warnf(config, "Failed to create the file %s\n", out->filename);
> + if (out->filename)
> + warnf(config, "Failed to create the file %s\n", out->filename);
> + else
> + warnf(config, "Remote filename has no length!\n");

The (out->filename) check belongs before the call of fopen() as the function
is not supposed to get NULL as path.

> +static size_t
> +header_callback(void *ptr, size_t size, size_t nmemb, void *stream)
> +{
> + struct OutStruct* outs = stream;
> +
> + if (curlx_strnequal(ptr, "Content-disposition:", 20)) {

if (20 < size*nmemb && curlx_strnequal(ptr, "Content-disposition:", 20)) {

> + filename = parse_filename(p, size*nmemb - (p - (char*)ptr));
> + if (filename) {
> + FILE* f = fopen(filename, "r");
> + if (f) {
> + fclose(f);
> + printf("\ncurl: Filename '%s' already exists. Aborting.\n",
> + filename);
> + outs->filename = NULL;
> + return -1;

The check above looks redundant to me. You in fact do not need to know whether
the file exists or not at this point. It does not even guarantee the file
won't exist at the time you create the new one anyhow.

The fopen() function should be called exactly once per one file. If you want
to provide some additional info (e.g. file already exists, insufficient
permissions to create file, etc.), you can use the error(3) function for that
purpose.

Kamil
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-users
FAQ: http://curl.haxx.se/docs/faq.html
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2009-12-30