[pacman-dev] [PATCH 1/6] dload: move curl option setting to static func
Dave Reisner
d at falconindy.com
Fri Aug 19 10:55:43 EDT 2011
On Fri, Aug 19, 2011 at 09:46:42AM -0500, Dan McGee wrote:
> On Thu, Aug 18, 2011 at 8:09 PM, Dave Reisner <d at falconindy.com> wrote:
> > Signed-off-by: Dave Reisner <dreisner at 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
> 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
> >
> >
> >
>
More information about the pacman-dev
mailing list