On Fri, Aug 19, 2011 at 09:46:42AM -0500, Dan McGee wrote:
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/dload.c | 112 ++++++++++++++++++++++++++++----------------------- 1 files changed, 62 insertions(+), 50 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 2a1fb41..16b2206 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -177,18 +177,74 @@ static size_t parse_headers(void *ptr, size_t size, size_t nmemb, void *user) return realsize; }
+static int curl_set_handle_opts(struct dload_payload *payload, + const char *destfile, const char *tempfile, FILE **localf, + const char **open_mode, char *error_buffer) +{ + char *useragent = getenv("HTTP_USER_AGENT"); + alpm_handle_t *handle = payload->handle; + struct stat st; + + /* the curl_easy handle is initialized with the alpm handle, so we only need + * to reset the curl handle set parameters for each time it's used. */ + curl_easy_reset(handle->curl); + curl_easy_setopt(handle->curl, CURLOPT_URL, payload->fileurl); + curl_easy_setopt(handle->curl, CURLOPT_FAILONERROR, 1L); + curl_easy_setopt(handle->curl, CURLOPT_ERRORBUFFER, error_buffer); + curl_easy_setopt(handle->curl, CURLOPT_CONNECTTIMEOUT, 10L); + curl_easy_setopt(handle->curl, CURLOPT_FILETIME, 1L); + curl_easy_setopt(handle->curl, CURLOPT_NOPROGRESS, 0L); + curl_easy_setopt(handle->curl, CURLOPT_FOLLOWLOCATION, 1L); + curl_easy_setopt(handle->curl, CURLOPT_PROGRESSFUNCTION, curl_progress); + curl_easy_setopt(handle->curl, CURLOPT_PROGRESSDATA, (void *)payload); + curl_easy_setopt(handle->curl, CURLOPT_LOW_SPEED_LIMIT, 1024L); + curl_easy_setopt(handle->curl, CURLOPT_LOW_SPEED_TIME, 10L); + curl_easy_setopt(handle->curl, CURLOPT_HEADERFUNCTION, parse_headers); + curl_easy_setopt(handle->curl, CURLOPT_WRITEHEADER, (void *)payload); + + if(payload->max_size) { + curl_easy_setopt(handle->curl, CURLOPT_MAXFILESIZE, payload->max_size); + } + + if(useragent != NULL) { + curl_easy_setopt(handle->curl, CURLOPT_USERAGENT, useragent); + } + + if(!payload->allow_resume && !payload->force && destfile && stat(destfile, &st) == 0) { + /* start from scratch, but only download if our local is out of date. */ + curl_easy_setopt(handle->curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE); + curl_easy_setopt(handle->curl, CURLOPT_TIMEVALUE, (long)st.st_mtime); + } else if(stat(tempfile, &st) == 0 && payload->allow_resume) { + /* a previous partial download exists, resume from end of file. */ + *open_mode = "ab"; + curl_easy_setopt(handle->curl, CURLOPT_RESUME_FROM, (long)st.st_size); + _alpm_log(handle, ALPM_LOG_DEBUG, "tempfile found, attempting continuation\n"); + payload->initial_size = (double)st.st_size; + } + + if(*localf == NULL) { + *localf = fopen(tempfile, *open_mode); + if(*localf == NULL) { + return 1; + } This is doing more than {subject} stated here. I'm not sure we should be opening files and such in this method. Is that only so the final setopt WRITEDATA call is moved here? I think it would make more sense to leave that one behind and greatly simplify the argument list of
On Thu, Aug 18, 2011 at 8:09 PM, Dave Reisner <d@falconindy.com> wrote: this method, rather than modifying stack variables of another function.
Fair point. Perhaps I went a bit overboard here, and I'm all for simplifying the param list a bit here. I've also considered globbing the tempfile, destfile, localf trifecta into a single struct (possibly attached to the payload), but I'm starting to run into variable name hell...
+ } + + curl_easy_setopt(handle->curl, CURLOPT_WRITEDATA, *localf); + + return 0; +} + + static int curl_download_internal(struct dload_payload *payload, const char *localpath, char **final_file) { int ret = -1; FILE *localf = NULL; - const char *useragent; const char *open_mode = "wb"; char *destfile = NULL, *tempfile = NULL, *effective_url; /* RFC1123 states applications should support this length */ char hostname[256]; - char error_buffer[CURL_ERROR_SIZE]; - struct stat st; + char error_buffer[CURL_ERROR_SIZE] = { 0 }; long timecond, remote_time = -1; double remote_size, bytes_dl; struct sigaction sig_pipe[2], sig_int[2]; @@ -233,55 +289,11 @@ static int curl_download_internal(struct dload_payload *payload, payload->filename = strrchr(randpath, '/') + 1; }
- error_buffer[0] = '\0'; - - /* the curl_easy handle is initialized with the alpm handle, so we only need - * to reset the curl handle set parameters for each time it's used. */ - curl_easy_reset(handle->curl); - curl_easy_setopt(handle->curl, CURLOPT_URL, payload->fileurl); - curl_easy_setopt(handle->curl, CURLOPT_FAILONERROR, 1L); - curl_easy_setopt(handle->curl, CURLOPT_ERRORBUFFER, error_buffer); - curl_easy_setopt(handle->curl, CURLOPT_CONNECTTIMEOUT, 10L); - curl_easy_setopt(handle->curl, CURLOPT_FILETIME, 1L); - curl_easy_setopt(handle->curl, CURLOPT_NOPROGRESS, 0L); - curl_easy_setopt(handle->curl, CURLOPT_FOLLOWLOCATION, 1L); - curl_easy_setopt(handle->curl, CURLOPT_PROGRESSFUNCTION, curl_progress); - curl_easy_setopt(handle->curl, CURLOPT_PROGRESSDATA, (void *)payload); - curl_easy_setopt(handle->curl, CURLOPT_LOW_SPEED_LIMIT, 1024L); - curl_easy_setopt(handle->curl, CURLOPT_LOW_SPEED_TIME, 10L); - curl_easy_setopt(handle->curl, CURLOPT_HEADERFUNCTION, parse_headers); - curl_easy_setopt(handle->curl, CURLOPT_WRITEHEADER, (void *)payload); - - if(payload->max_size) { - curl_easy_setopt(handle->curl, CURLOPT_MAXFILESIZE, payload->max_size); - } - - useragent = getenv("HTTP_USER_AGENT"); - if(useragent != NULL) { - curl_easy_setopt(handle->curl, CURLOPT_USERAGENT, useragent); - } - - if(!payload->allow_resume && !payload->force && destfile && stat(destfile, &st) == 0) { - /* start from scratch, but only download if our local is out of date. */ - curl_easy_setopt(handle->curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE); - curl_easy_setopt(handle->curl, CURLOPT_TIMEVALUE, (long)st.st_mtime); - } else if(stat(tempfile, &st) == 0 && payload->allow_resume) { - /* a previous partial download exists, resume from end of file. */ - open_mode = "ab"; - curl_easy_setopt(handle->curl, CURLOPT_RESUME_FROM, (long)st.st_size); - _alpm_log(handle, ALPM_LOG_DEBUG, "tempfile found, attempting continuation\n"); - payload->initial_size = (double)st.st_size; - } - - if(localf == NULL) { - localf = fopen(tempfile, open_mode); - if(localf == NULL) { - goto cleanup; - } + if(curl_set_handle_opts(payload, destfile, tempfile, &localf, &open_mode, + error_buffer) != 0) { + goto cleanup; }
- curl_easy_setopt(handle->curl, CURLOPT_WRITEDATA, localf); - /* ignore any SIGPIPE signals- these may occur if our FTP socket dies or * something along those lines. Store the old signal handler first. */ sig_pipe[NEW].sa_handler = SIG_IGN; -- 1.7.6