On Fri, Jul 08, 2011 at 09:59:30PM +1000, Allan McRae wrote:
> From: Ivan Kanakarakis <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
>
