On Fri, Jul 08, 2011 at 09:59:30PM +1000, Allan McRae wrote:
From: Ivan Kanakarakis <ivan.kanak@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.
Perhaps this is just academic, but is there a test case for this? I'm not aware of any situation where this it's necessary to quote to assignment of a command sub or simple variable to a var to avoid any oddities. $ foo= bar="" baz=${notavar} $ echo ${#foo} ${#bar} ${#baz} 0 0 0
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com> --- scripts/pacman-key.sh.in | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index d75c111..b15a3a5 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -137,12 +137,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" @@ -152,12 +152,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 @@ -168,7 +168,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 @@ -179,7 +179,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 @@ -264,7 +264,7 @@ if [[ ! -r "${CONFIG}" ]]; then fi
# Get GPGDIR from pacman.conf iff not specified on command line -if [[ -z PACMAN_KEYRING_DIR && GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then +if [[ -z PACMAN_KEYRING_DIR && GPGDIR="$(get_from "$CONFIG" "GPGDir")" == 0 ]]; then
This doesn't pertain to this patch, but I don't understand this logic. get_from should be writing the value of GPGDir as it's read from $CONFIG. It looks like the goal here was to make sure that get_from was successful, which would be written as: if [[ -z PACMAN_KEYRING_DIR ]] && GPGDIR=$(get_from "$CONFIG" "GPGDir"); then PACMAN_KEYRING_DIR=$GPGDIR fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg} Or through a single default assignment to tidy the whole thing up: # if PACMAN_KEYRING_DIR isn't assigned, try to get it from the config # file, falling back on a hard default. : ${PACMAN_KEYRING_DIR:=$(get_from "$CONFIG" "GPGDir" || echo "@sysconfdir@/pacman.d/gnupg")} Will happily write up a patch if this is what was actually intended... d
PACMAN_KEYRING_DIR="${GPGDIR}" fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg} -- 1.7.6