[pacman-dev] [PATCH v5a] Add configuration options for libcurl's "low speed" timeout
Allan McRae
allan at archlinux.org
Sun May 4 02:10:30 EDT 2014
On 02/04/14 16:59, Andrew Hills wrote:
> 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.
>
> Signed-off-by: Andrew Hills <ahills at ednos.net>
> ---
>
> This version of the patch incorporates Dave Reisner's guidance regarding code
> style, string translation considerations, and error handling for invalid
> inputs to the new options. The new utility function is in util.[ch]. I opted
> for brevity in the error handling.
>
> This minor update is an attempt to cover up a silly errno mistake. Sorry for
> the extra noise.
Sorry for the delay. Fairly cosmetic comments below and then it looks
good to me.
Allan
> doc/pacman.conf.5.txt | 13 +++++++++++++
> etc/pacman.conf.in | 2 ++
> lib/libalpm/alpm.h | 10 ++++++++++
> lib/libalpm/dload.c | 4 ++--
> lib/libalpm/handle.c | 32 ++++++++++++++++++++++++++++++++
> lib/libalpm/handle.h | 4 ++++
> src/pacman/conf.c | 24 ++++++++++++++++++++++++
> src/pacman/conf.h | 4 ++++
> src/pacman/util.c | 19 +++++++++++++++++++
> src/pacman/util.h | 1 +
> 10 files changed, 111 insertions(+), 2 deletions(-)
>
> diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt
> index f19311c..bfc0191 100644
> --- a/doc/pacman.conf.5.txt
> +++ b/doc/pacman.conf.5.txt
> @@ -199,6 +199,19 @@ 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
> -------------------
> Each repository section defines a section name and at least one location where
> diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in
> index b891de1..0bc5908 100644
> --- a/etc/pacman.conf.in
> +++ b/etc/pacman.conf.in
> @@ -20,6 +20,8 @@ HoldPkg = pacman glibc
> #CleanMethod = KeepInstalled
> #UseDelta = 0.7
> Architecture = auto
Have these in their own section before "# Misc options". Give the
header "# Downloader options".
> +#LowSpeedLimit = 1
> +#LowSpeedTime = 10
>
> # Pacman won't upgrade packages listed in IgnorePkg and members of IgnoreGroup
> #IgnorePkg =
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index b0adb95..d23ae23 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -595,6 +595,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_lowspeedlimit(alpm_handle_t *handle);
I'd like these clarified that it is download configuration options.
alpm_option_get_download_lowspeedlimit
> +/** Sets the libcurl low speed limit in bytes per second. */
> +int alpm_option_set_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit);
> +
> +/** Returns the libcurl low speed time in seconds. */
> +long alpm_option_get_lowspeedtime(alpm_handle_t *handle);
> +/** Sets the libcurl low speed time in seconds. */
> +int alpm_option_set_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 53867f5..31edbf4 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -294,8 +294,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->lowspeedlimit);
> + curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, handle->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 0842d51..b7a13a0 100644
> --- a/lib/libalpm/handle.c
> +++ b/lib/libalpm/handle.c
> @@ -198,6 +198,18 @@ const char SYMEXPORT *alpm_option_get_lockfile(alpm_handle_t *handle)
> return handle->lockfile;
> }
>
> +long SYMEXPORT alpm_option_get_lowspeedlimit(alpm_handle_t *handle)
> +{
> + CHECK_HANDLE(handle, return -1);
> + return handle->lowspeedlimit;
> +}
> +
> +long SYMEXPORT alpm_option_get_lowspeedtime(alpm_handle_t *handle)
> +{
> + CHECK_HANDLE(handle, return -1);
> + return handle->lowspeedtime;
> +}
> +
> const char SYMEXPORT *alpm_option_get_gpgdir(alpm_handle_t *handle)
> {
> CHECK_HANDLE(handle, return NULL);
> @@ -294,6 +306,26 @@ int SYMEXPORT alpm_option_set_progresscb(alpm_handle_t *handle, alpm_cb_progress
> return 0;
> }
>
> +int SYMEXPORT alpm_option_set_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit)
> +{
> + CHECK_HANDLE(handle, return -1);
> + if(lowspeedlimit < 0) {
> + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);
> + }
> + handle->lowspeedlimit = lowspeedlimit;
> + return 0;
> +}
> +
> +int SYMEXPORT alpm_option_set_lowspeedtime(alpm_handle_t *handle, long lowspeedtime)
> +{
> + CHECK_HANDLE(handle, return -1);
> + if(lowspeedtime < 0) {
> + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);
> + }
> + handle->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 27241ea..25a51ff 100644
> --- a/lib/libalpm/handle.h
> +++ b/lib/libalpm/handle.h
> @@ -104,6 +104,10 @@ struct __alpm_handle_t {
> /* for delta parsing efficiency */
> int delta_regex_compiled;
> regex_t delta_regex;
> +
> + /* Curl timeouts */
> + long lowspeedlimit;
> + long lowspeedtime;
In a previous patch it was suggested that these parameters are put into
a struct so that the number of members in the handle did not increase if
we allowed more of these to be adjusted. I'm going to require that...
Call it something like "download".
> };
>
> alpm_handle_t *_alpm_handle_new(void);
> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
> index f75f3a7..0032ce0 100644
> --- a/src/pacman/conf.c
> +++ b/src/pacman/conf.c
> @@ -120,6 +120,9 @@ config_t *config_new(void)
> newconfig->colstr.err = "";
> newconfig->colstr.nocolor = "";
>
> + newconfig->lowspeedlimit = 1L;
> + newconfig->lowspeedtime = 10L;
> +
> return newconfig;
> }
>
> @@ -596,6 +599,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"),
> @@ -690,6 +711,9 @@ static int setup_libalpm(void)
> alpm_option_set_questioncb(handle, cb_question);
> alpm_option_set_progresscb(handle, cb_progress);
>
> + alpm_option_set_lowspeedlimit(handle, config->lowspeedlimit);
> + alpm_option_set_lowspeedtime(handle, config->lowspeedtime);
> +
> config->logfile = config->logfile ? config->logfile : strdup(LOGFILE);
> ret = alpm_option_set_logfile(handle, config->logfile);
> if(ret != 0) {
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index e8cac50..3f2cdb3 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -113,6 +113,10 @@ typedef struct __config_t {
>
> /* Color strings for output */
> colstr_t colstr;
> +
> + /* Curl timeouts */
> + long lowspeedlimit;
> + long lowspeedtime;
> } config_t;
>
> /* Operations */
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index d42e27b..1c7fb0f 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -1535,6 +1535,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 4a31e89..2f0271f 100644
> --- a/src/pacman/util.h
> +++ b/src/pacman/util.h
> @@ -74,6 +74,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)));
>
More information about the pacman-dev
mailing list