[pacman-dev] [PATCH v6 1/1] Add configuration options for libcurl's "low speed" timeout

Dave Reisner d at falconindy.com
Sun Nov 13 23:58:27 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.
> 
> 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>
> ---

I'm going to restate my prior objection to this patch series, as
presented:

https://lists.archlinux.org/pipermail/pacman-dev/2016-August/021283.html
https://lists.archlinux.org/pipermail/pacman-dev/2016-September/021367.html

What you want is to provide an *interface*, not a 1:1 mapping of curl
internals to public pacman options.

>  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