On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote: > From: Christian Hesse <[email protected]> > > Add LowSpeedLimit and LowSpeedTime configuration options to correspond > to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. > This allows, e.g., transfers behind corporate virus-scanning firewalls > to survive the delays. Increasing the timeout may not be desirable in > all situations; similarly, disabling the check prevents detection of > disappearing networks.
FWIW, I'm strongly opposed to having a 1:1 mapping between pacman options and curl config options. Please look at the bigger picture -- it's dead connection detection. We might reimplement that in the future in some other way (e.g. via the progress callback or by some other transfer library entirely). To that end, I think it would be reasonable to add a boolean toggle for the dead connection detection (default on). The patch in its current state makes me rather itchy from an API perspective. > Adds a utility function, parse_positive_long(), which yields a positive > (but not unsigned) long integer from a null-terminated string using > strtol(). A string containing anything but a base-10 number will cause > the function to return -1. > > Original-Patch-by: Andrew Hills <[email protected]> > Signed-off-by: Christian Hesse <[email protected]> > --- > doc/pacman.conf.5.txt | 12 ++++++++++++ > etc/pacman.conf.in | 4 ++++ > lib/libalpm/alpm.h | 10 ++++++++++ > lib/libalpm/dload.c | 4 ++-- > lib/libalpm/handle.c | 32 ++++++++++++++++++++++++++++++++ > lib/libalpm/handle.h | 9 +++++++++ > src/pacman/conf.c | 24 ++++++++++++++++++++++++ > src/pacman/conf.h | 4 ++++ > src/pacman/util.c | 19 +++++++++++++++++++ > src/pacman/util.h | 1 + > 10 files changed, 117 insertions(+), 2 deletions(-) > > diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt > index c665870..2346331 100644 > --- a/doc/pacman.conf.5.txt > +++ b/doc/pacman.conf.5.txt > @@ -209,6 +209,18 @@ Options > Displays name, version and size of target packages formatted > as a table for upgrade, sync and remove operations. > > +*LowSpeedLimit* = speed:: > + Sets the speed in bytes per second that a download should be below during > + `LowSpeedTime` seconds to abort the transfer for being too slow. Setting > + 'speed' to 0 will disable the speed check. Defaults to 1 byte per second. > + Note that this option will not affect external programs specified by > + `XferCommand`. > + > +*LowSpeedTime* = time:: > + Sets the time in seconds that a download should be below the > `LowSpeedLimit` > + transfer speed to abort the transfer for being too slow. Setting 'time' > to > + 0 will disable the speed check. Defaults to 10 seconds. Note that this > + option will not affect external programs specified by `XferCommand`. > > Repository Sections > ------------------- > diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in > index 53071e5..ddc7397 100644 > --- a/etc/pacman.conf.in > +++ b/etc/pacman.conf.in > @@ -29,6 +29,10 @@ Architecture = auto > #NoUpgrade = > #NoExtract = > > +# Downloader options > +#LowSpeedLimit = 1 > +#LowSpeedTime = 10 > + > # Misc options > #UseSyslog > #Color > diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h > index 168d71b..c338119 100644 > --- a/lib/libalpm/alpm.h > +++ b/lib/libalpm/alpm.h > @@ -811,6 +811,16 @@ const char *alpm_option_get_dbpath(alpm_handle_t > *handle); > /** Get the name of the database lock file. Read-only. */ > const char *alpm_option_get_lockfile(alpm_handle_t *handle); > > +/** Returns the libcurl low speed limit in bytes per second. */ > +long alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle); > +/** Sets the libcurl low speed limit in bytes per second. */ > +int alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long > lowspeedlimit); > + > +/** Returns the libcurl low speed time in seconds. */ > +long alpm_option_get_download_lowspeedtime(alpm_handle_t *handle); > +/** Sets the libcurl low speed time in seconds. */ > +int alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long > lowspeedtime); > + > /** @name Accessors to the list of package cache directories. > * @{ > */ > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c > index f4e6a27..7490dd6 100644 > --- a/lib/libalpm/dload.c > +++ b/lib/libalpm/dload.c > @@ -305,8 +305,8 @@ static void curl_set_handle_opts(struct dload_payload > *payload, > curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); > curl_easy_setopt(curl, CURLOPT_PROGRESSFUNCTION, dload_progress_cb); > curl_easy_setopt(curl, CURLOPT_PROGRESSDATA, (void *)payload); > - curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, 1L); > - curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, 10L); > + curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, > handle->download.lowspeedlimit); > + curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, > handle->download.lowspeedtime); > curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, dload_parseheader_cb); > curl_easy_setopt(curl, CURLOPT_WRITEHEADER, (void *)payload); > curl_easy_setopt(curl, CURLOPT_NETRC, CURL_NETRC_OPTIONAL); > diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c > index e9439a0..8cfe0a1 100644 > --- a/lib/libalpm/handle.c > +++ b/lib/libalpm/handle.c > @@ -247,6 +247,18 @@ const char SYMEXPORT > *alpm_option_get_lockfile(alpm_handle_t *handle) > return handle->lockfile; > } > > +long SYMEXPORT alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle) > +{ > + CHECK_HANDLE(handle, return -1); > + return handle->download.lowspeedlimit; > +} > + > +long SYMEXPORT alpm_option_get_download_lowspeedtime(alpm_handle_t *handle) > +{ > + CHECK_HANDLE(handle, return -1); > + return handle->download.lowspeedtime; > +} > + > const char SYMEXPORT *alpm_option_get_gpgdir(alpm_handle_t *handle) > { > CHECK_HANDLE(handle, return NULL); > @@ -362,6 +374,26 @@ int SYMEXPORT alpm_option_set_progresscb(alpm_handle_t > *handle, alpm_cb_progress > return 0; > } > > +int SYMEXPORT alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, > long lowspeedlimit) > +{ > + CHECK_HANDLE(handle, return -1); > + if(lowspeedlimit < 0) { > + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); > + } > + handle->download.lowspeedlimit = lowspeedlimit; > + return 0; > +} > + > +int SYMEXPORT alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, > long lowspeedtime) > +{ > + CHECK_HANDLE(handle, return -1); > + if(lowspeedtime < 0) { > + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); > + } > + handle->download.lowspeedtime = lowspeedtime; > + return 0; > +} > + > static char *canonicalize_path(const char *path) > { > char *new_path; > diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h > index a1d0f9a..73fa584 100644 > --- a/lib/libalpm/handle.h > +++ b/lib/libalpm/handle.h > @@ -50,6 +50,14 @@ do { \ > } \ > } while(0) > > +#ifdef HAVE_LIBCURL > +struct download { > + /* Curl timeouts */ > + long lowspeedlimit; > + long lowspeedtime; > +}; > +#endif > + > struct __alpm_handle_t { > /* internal usage */ > alpm_db_t *db_local; /* local db pointer */ > @@ -60,6 +68,7 @@ struct __alpm_handle_t { > #ifdef HAVE_LIBCURL > /* libcurl handle */ > CURL *curl; /* reusable curl_easy handle */ > + struct download download; > #endif > > #ifdef HAVE_LIBGPGME > diff --git a/src/pacman/conf.c b/src/pacman/conf.c > index 25de7af..21db633 100644 > --- a/src/pacman/conf.c > +++ b/src/pacman/conf.c > @@ -122,6 +122,9 @@ config_t *config_new(void) > newconfig->colstr.err = ""; > newconfig->colstr.nocolor = ""; > > + newconfig->lowspeedlimit = 1L; > + newconfig->lowspeedtime = 10L; > + > return newconfig; > } > > @@ -604,6 +607,24 @@ static int _parse_options(const char *key, char *value, > return 1; > } > FREELIST(values); > + } else if(strcmp(key, "LowSpeedLimit") == 0) { > + config->lowspeedlimit = parse_positive_long(value); > + if(config->lowspeedlimit < 0) { > + pm_printf(ALPM_LOG_WARNING, > + _("config file %s, line %d: > invalid %s, using default.\n"), > + file, linenum, "LowSpeedLimit"); > + config->lowspeedlimit = 1L; > + } > + pm_printf(ALPM_LOG_DEBUG, "config: lowspeedlimit: > %ld\n", config->lowspeedlimit); > + } else if(strcmp(key, "LowSpeedTime") == 0) { > + config->lowspeedtime = parse_positive_long(value); > + if(config->lowspeedtime < 0) { > + pm_printf(ALPM_LOG_WARNING, > + _("config file %s, line %d: > invalid %s, using default.\n"), > + file, linenum, "LowSpeedTime"); > + config->lowspeedtime = 10L; > + } > + pm_printf(ALPM_LOG_DEBUG, "config: lowspeedtime: > %ld\n", config->lowspeedtime); > } else { > pm_printf(ALPM_LOG_WARNING, > _("config file %s, line %d: directive > '%s' in section '%s' not recognized.\n"), > @@ -734,6 +755,9 @@ static int setup_libalpm(void) > alpm_option_set_questioncb(handle, cb_question); > alpm_option_set_progresscb(handle, cb_progress); > > + alpm_option_set_download_lowspeedlimit(handle, config->lowspeedlimit); > + alpm_option_set_download_lowspeedtime(handle, config->lowspeedtime); > + > if(config->op == PM_OP_FILES) { > alpm_option_set_dbext(handle, ".files"); > } > diff --git a/src/pacman/conf.h b/src/pacman/conf.h > index 2aba8bf..c61ed2a 100644 > --- a/src/pacman/conf.h > +++ b/src/pacman/conf.h > @@ -131,6 +131,10 @@ typedef struct __config_t { > /* Color strings for output */ > colstr_t colstr; > > + /* Curl timeouts */ > + long lowspeedlimit; > + long lowspeedtime; > + > alpm_list_t *repos; > } config_t; > > diff --git a/src/pacman/util.c b/src/pacman/util.c > index b979083..b1875bd 100644 > --- a/src/pacman/util.c > +++ b/src/pacman/util.c > @@ -1620,6 +1620,25 @@ int noyes(const char *format, ...) > return ret; > } > > +/* Attempts to extract a positive long from a string; failure yields -1. */ > +long parse_positive_long(const char *str) > +{ > + char *end; > + long num; > + > + errno = 0; > + num = strtol(str, &end, 10); > + > + if(end == str || > /* Not a decimal number */ > + *end != 0 || > /* Extra characters */ > + (num == LONG_MAX && errno == ERANGE) || /* Out of range */ > + num < 0) { > /* Out of range (negative) */ > + return -1; > + } > + > + return num; > +} > + > int colon_printf(const char *fmt, ...) > { > int ret; > diff --git a/src/pacman/util.h b/src/pacman/util.h > index f5e37c8..d3eb77f 100644 > --- a/src/pacman/util.h > +++ b/src/pacman/util.h > @@ -75,6 +75,7 @@ int multiselect_question(char *array, int count); > int colon_printf(const char *format, ...) __attribute__((format(printf, 1, > 2))); > int yesno(const char *format, ...) __attribute__((format(printf, 1, 2))); > int noyes(const char *format, ...) __attribute__((format(printf, 1, 2))); > +long parse_positive_long(const char *str); > > int pm_printf(alpm_loglevel_t level, const char *format, ...) > __attribute__((format(printf,2,3))); > int pm_asprintf(char **string, const char *format, ...) > __attribute__((format(printf,2,3))); > -- > 2.9.3
