[pacman-dev] [PATCH 4/7] pacman-key: fix quotation on several variable assignments

[Dan McGee]

On Thu, Apr 21, 2011 at 8:59 AM,  <ivan.kanak at gmail.com> wrote:
> From: Ivan Kanakarakis <ivan.kanak at gmail.com>
>
> This commit adds quotes to several variable assignments. Unquoted values
> can cause problems on several occasions if the value is empty. It is
> safer to have every assignment quoted.
>
> Signed-off-by: Ivan Kanakarakis <ivan.kanak at gmail.com>
> ---
>  scripts/pacman-key.sh.in |   24 ++++++++++++------------
>  1 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index c092989..7b9c853 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -27,23 +27,23 @@ export TEXTDOMAINDIR='@localedir@'
>  myver="@PACKAGE_VERSION@"
>
>  msg() {
> -       local mesg=$1; shift
> +       local mesg="$1"; shift
>        printf "==> ${mesg}\n" "$@" >&1
>  }
>
>  msg2() {
>        (( QUIET )) && return
> -       local mesg=$1; shift
> +       local mesg="$1"; shift
>        printf "  -> ${mesg}\n" "$@" >&1
>  }
>
>  warning() {
> -       local mesg=$1; shift
> +       local mesg="$1"; shift
>        printf "==> $(gettext "WARNING:") ${mesg}\n" "$@" >&2
>  }
>
>  error() {
> -       local mesg=$1; shift
> +       local mesg="$1"; shift
>        printf "==> $(gettext "ERROR:") ${mesg}\n" "$@" >&2
>  }

First, I don't think these ones are even necessary. Second, this
doesn't belong in this commit if so, as you have several other places
that would need to be fixed cross-script as these are common methods.

dmcgee at galway ~/projects/pacman (master)
$ git grep 'mesg=' | cat
contrib/pacscripts.in:  local mesg=$1; shift
scripts/makepkg.sh.in:  local mesg=$1; shift
scripts/makepkg.sh.in:  local mesg=$1; shift
scripts/makepkg.sh.in:  local mesg=$1; shift
scripts/makepkg.sh.in:  local mesg=$1; shift
scripts/makepkg.sh.in:  local mesg=$1; shift
scripts/pacman-db-upgrade.sh.in:        local mesg=$1; shift
scripts/pacman-db-upgrade.sh.in:        local mesg=$1; shift
scripts/pacman-key.sh.in:       local mesg=$1; shift
scripts/pacman-key.sh.in:       local mesg=$1; shift
scripts/pacman-key.sh.in:       local mesg=$1; shift
scripts/pacman-key.sh.in:       local mesg=$1; shift
scripts/pacman-optimize.sh.in:  local mesg=$1; shift
scripts/pacman-optimize.sh.in:  local mesg=$1; shift
scripts/pkgdelta.sh.in: local mesg=$1; shift
scripts/pkgdelta.sh.in: local mesg=$1; shift
scripts/pkgdelta.sh.in: local mesg=$1; shift
scripts/repo-add.sh.in: local mesg=$1; shift
scripts/repo-add.sh.in: local mesg=$1; shift
scripts/repo-add.sh.in: local mesg=$1; shift
scripts/repo-add.sh.in: local mesg=$1; shift

>
> @@ -145,12 +145,12 @@ reload_keyring() {
>        if [[ -r "${REMOVED_KEYS}" ]]; then
>                while read key; do
>                        local key_values name
> -                       key_values=$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5,10 --output-delimiter=' ')
> +                       key_values="$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5,10 --output-delimiter=' ')"
>                        if [[ -n $key_values ]]; then
>                                # The first word is the key_id
> -                               key_id=${key_values%% *}
> +                               key_id="${key_values%% *}"
>                                # the rest if the name of the owner
> -                               name=${key_values#* }
> +                               name="${key_values#* }"
>                                if [[ -n ${key_id} ]]; then
>                                        # Mark this key to be deleted
>                                        removed_ids[$key_id]="$name"
> @@ -160,12 +160,12 @@ reload_keyring() {
>        fi
>
>        # List of keys that must be kept installed, even if in the list of keys to be removed
> -       local HOLD_KEYS=$(get_from "$CONFIG" "HoldKeys")
> +       local HOLD_KEYS="$(get_from "$CONFIG" "HoldKeys")"
>
>        # Remove the keys that must be kept from the set of keys that should be removed
>        if [[ -n ${HOLD_KEYS} ]]; then
>                for key in ${HOLD_KEYS}; do
> -                       key_id=$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5)
> +                       key_id="$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5)"
>                        if [[ -n "${removed_ids[$key_id]}" ]]; then
>                                unset removed_ids[$key_id]
>                        fi
> @@ -176,7 +176,7 @@ reload_keyring() {
>        # be updated automatically.
>        if [[ -r "${ADDED_KEYS}" ]]; then
>                msg "$(gettext "Appending official keys...")"
> -               local add_keys=$(${GPG_NOKEYRING} --keyring "${ADDED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5)
> +               local add_keys="$(${GPG_NOKEYRING} --keyring "${ADDED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5)"
>                for key_id in ${add_keys}; do
>                        # There is no point in adding a key that will be deleted right after
>                        if [[ -z "${removed_ids[$key_id]}" ]]; then
> @@ -187,7 +187,7 @@ reload_keyring() {
>
>        if [[ -r "${DEPRECATED_KEYS}" ]]; then
>                msg "$(gettext "Appending deprecated keys...")"
> -               local add_keys=$(${GPG_NOKEYRING} --keyring "${DEPRECATED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5)
> +               local add_keys="$(${GPG_NOKEYRING} --keyring "${DEPRECATED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5)"
>                for key_id in ${add_keys}; do
>                        # There is no point in adding a key that will be deleted right after
>                        if [[ -z "${removed_ids[$key_id]}" ]]; then
> @@ -245,7 +245,7 @@ if [[ ! -r "${CONFIG}" ]]; then
>  fi
>
>  # Read GPGDIR from $CONFIG.
> -if [[ GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then
> +if [[ GPGDIR="$(get_from "$CONFIG" "GPGDir")" == 0 ]]; then
>        PACMAN_KEYRING_DIR="${GPGDIR}"
>  fi
>  GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning"

No objection to the rest of it, however.

-Dan