On 10/5/20 2:32 pm, Eli Schwartz wrote:
> This is not a warning, _parse_options() returns failure without even
> parsing further lines and the attempted pacman/pacman-conf program
> execution immediately aborts. Warnings are for when e.g. later on if we
> don't recognize a setting at all, we skip over it and have enough
> confidence in this to continue executing the program.
>
> The current implementation results in pacman-conf aborting with:
>
> warning: config file /etc/pacman.conf, line 60: invalid value for
> 'ParallelDownloads' : '2.5'
> error parsing '/etc/pacman.conf'
>
> or pacman -Syu aborting with the entirely more cryptic:
>
> warning: config file /etc/pacman.conf, line 59: invalid value for
> 'ParallelDownloads' : '2.5'
>
> and this isn't just a problem for the newly added ParallelDownloads
> setting, either, you could get the same problem if you specified a
> broken XferCommand, but that's harder as it's more accepting of input
> and you probably don't hit this except with unbalanced quotes.
>
Is this the fix we want? I think it would be better to get to the end
of parsing the conf file spitting errors as we go so that all issues are
printed before exiting.
Allan
> Signed-off-by: Eli Schwartz <eschwa...@archlinux.org>
> ---
> src/pacman/conf.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
> index ac5a5329..3a3ef605 100644
> --- a/src/pacman/conf.c
> +++ b/src/pacman/conf.c
> @@ -665,7 +665,7 @@ static int _parse_options(const char *key, char *value,
> } else if(strcmp(key, "XferCommand") == 0) {
> char **c;
> if((config->xfercommand_argv = wordsplit(value)) ==
> NULL) {
> - pm_printf(ALPM_LOG_WARNING,
> + pm_printf(ALPM_LOG_ERROR,
> _("config file %s, line %d:
> invalid value for '%s' : '%s'\n"),
> file, linenum, "XferCommand",
> value);
> return 1;
> @@ -717,21 +717,21 @@ static int _parse_options(const char *key, char *value,
>
> err = parse_number(value, &number);
> if(err) {
> - pm_printf(ALPM_LOG_WARNING,
> + pm_printf(ALPM_LOG_ERROR,
> _("config file %s, line %d:
> invalid value for '%s' : '%s'\n"),
> file, linenum,
> "ParallelDownloads", value);
> return 1;
> }
>
> if(number < 1) {
> - pm_printf(ALPM_LOG_WARNING,
> + pm_printf(ALPM_LOG_ERROR,
> _("config file %s, line %d:
> value for '%s' has to be positive : '%s'\n"),
> file, linenum,
> "ParallelDownloads", value);
> return 1;
> }
>
> if(number > INT_MAX) {
> - pm_printf(ALPM_LOG_WARNING,
> + pm_printf(ALPM_LOG_ERROR,
> _("config file %s, line %d:
> value for '%s' is too large : '%s'\n"),
> file, linenum,
> "ParallelDownloads", value);
> return 1;
>
