[pacman-dev] [PATCH v6 1/1] Add configuration options for libcurl's "low speed" timeout
Dave Reisner
d at falconindy.com
Tue Aug 30 12:46:08 UTC 2016
On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
>
> 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 <ahills at ednos.net>
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
> 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
More information about the pacman-dev
mailing list