Using 'local' in a safe way.

First of all, 'local' is non-POSIX, but it is a really good safeguard
against hard-to-find bugs. However, doing a local + initializing combo
like `local X=$Y` in dash is error prone. If `Y=1 2` will get an error
since dash will expand $Y so we get `local X=1 2`, but it will treat
the "2" as another variable to be made local, which isn't what we
want. Hence, let's declare variables local and initialize them in
separate commands, which is safe.

diff --git a/scripts/boot/9990-misc-helpers.sh b/scripts/boot/9990-misc-helpers.sh
index 94c48e6..35448b9 100755 (executable)
--- a/scripts/boot/9990-misc-helpers.sh
+++ b/scripts/boot/9990-misc-helpers.sh
@@ -65,7 +65,8 @@ get_backing_device ()
 match_files_in_dir ()
 {
        # Does any files match pattern ${1} ?
-       local pattern="${1}"
+       local pattern
+       pattern="${1}"
 
        if [ "$(echo ${pattern})" != "${pattern}" ]
        then
@@ -378,24 +379,27 @@ really_export ()
 
 is_in_list_separator_helper ()
 {
-       local sep=${1}
+       local sep element list
+       sep=${1}
        shift
-       local element=${1}
+       element=${1}
        shift
-       local list=${*}
+       list=${*}
        echo ${list} | grep -qe "^\(.*${sep}\)\?${element}\(${sep}.*\)\?$"
 }
 
 is_in_space_sep_list ()
 {
-       local element=${1}
+       local element
+       element=${1}
        shift
        is_in_list_separator_helper "[[:space:]]" "${element}" "${*}"
 }
 
 is_in_comma_sep_list ()
 {
-       local element=${1}
+       local element
+       element=${1}
        shift
        is_in_list_separator_helper "," "${element}" "${*}"
 }
@@ -512,25 +516,28 @@ trim_path ()
 
 what_is_mounted_on ()
 {
-       local dir="$(trim_path ${1})"
+       local dir
+       dir="$(trim_path ${1})"
        grep -m1 "^[^ ]\+ ${dir} " /proc/mounts | cut -d' ' -f1
 }
 
 chown_ref ()
 {
-       local reference="${1}"
+       local reference targets owner
+       reference="${1}"
        shift
-       local targets=${@}
-       local owner=$(stat -c %u:%g "${reference}")
+       targets=${@}
+       owner=$(stat -c %u:%g "${reference}")
        chown -h ${owner} ${targets}
 }
 
 chmod_ref ()
 {
-       local reference="${1}"
+       local reference targets rights
+       reference="${1}"
        shift
-       local targets=${@}
-       local rights=$(stat -c %a "${reference}")
+       targets=${@}
+       rights=$(stat -c %a "${reference}")
        chmod ${rights} ${targets}
 }
 
@@ -616,12 +623,13 @@ load_keymap ()
 
 setup_loop ()
 {
-       local fspath=${1}
-       local module=${2}
-       local pattern=${3}
-       local offset=${4}
-       local encryption=${5}
-       local readonly=${6}
+       local fspath module pattern offset encryption readonly
+       fspath=${1}
+       module=${2}
+       pattern=${3}
+       offset=${4}
+       encryption=${5}
+       readonly=${6}
 
        # the output of setup_loop is evaluated in other functions,
        # modprobe leaks kernel options like "libata.dma=0"
@@ -730,17 +738,18 @@ try_mount ()
 # success, print the mount point for $device.
 mount_persistence_media ()
 {
-       local device=${1}
-       local probe=${2}
+       local device probe backing old_backing fstype mount_opts
+       device=${1}
+       probe=${2}
 
-       local backing="${rootmnt}/lib/live/mount/persistence/$(basename ${device})"
+       backing="${rootmnt}/lib/live/mount/persistence/$(basename ${device})"
 
        mkdir -p "${backing}"
-       local old_backing="$(where_is_mounted ${device})"
+       old_backing="$(where_is_mounted ${device})"
        if [ -z "${old_backing}" ]
        then
-               local fstype="$(get_fstype ${device})"
-               local mount_opts="rw,noatime"
+               fstype="$(get_fstype ${device})"
+               mount_opts="rw,noatime"
                if [ -n "${PERSISTENCE_READONLY}" ]
                then
                        mount_opts="ro,noatime"
@@ -775,8 +784,9 @@ mount_persistence_media ()
 
 close_persistence_media ()
 {
-       local device=${1}
-       local backing="$(where_is_mounted ${device})"
+       local device backing
+       device=${1}
+       backing="$(where_is_mounted ${device})"
 
        if [ -d "${backing}" ]
        then
@@ -842,22 +852,25 @@ open_luks_device ()
 
 get_gpt_name ()
 {
-    local dev="${1}"
+    local dev
+    dev="${1}"
     /sbin/blkid -s PART_ENTRY_NAME -p -o value ${dev} 2>/dev/null
 }
 
 is_gpt_device ()
 {
-    local dev="${1}"
+    local dev
+    dev="${1}"
     [ "$(/sbin/blkid -s PART_ENTRY_SCHEME -p -o value ${dev} 2>/dev/null)" = "gpt" ]
 }
 
 probe_for_gpt_name ()
 {
-       local overlays="${1}"
-       local dev="${2}"
+       local overlays dev gpt_dev gpt_name
+       overlays="${1}"
+       dev="${2}"
 
-       local gpt_dev="${dev}"
+       gpt_dev="${dev}"
        if is_active_luks_mapping ${dev}
        then
                # if $dev is an opened luks device, we need to check
@@ -870,7 +883,7 @@ probe_for_gpt_name ()
                return
        fi
 
-       local gpt_name=$(get_gpt_name ${gpt_dev})
+       gpt_name=$(get_gpt_name ${gpt_dev})
        for label in ${overlays}
        do
                if [ "${gpt_name}" = "${label}" ]
@@ -882,8 +895,9 @@ probe_for_gpt_name ()
 
 probe_for_fs_label ()
 {
-       local overlays="${1}"
-       local dev="${2}"
+       local overlays dev
+       overlays="${1}"
+       dev="${2}"
 
        for label in ${overlays}
        do
@@ -896,11 +910,12 @@ probe_for_fs_label ()
 
 probe_for_file_name ()
 {
-       local overlays="${1}"
-       local dev="${2}"
+       local overlays dev ret backing
+       overlays="${1}"
+       dev="${2}"
 
-       local ret=""
-       local backing="$(mount_persistence_media ${dev} probe)"
+       ret=""
+       backing="$(mount_persistence_media ${dev} probe)"
        if [ -z "${backing}" ]
        then
            return
@@ -911,7 +926,8 @@ probe_for_file_name ()
                path=${backing}/${PERSISTENCE_PATH}${label}
                if [ -f "${path}" ]
                then
-                       local loopdev=$(setup_loop "${path}" "loop" "/sys/block/loop*")
+                       local loopdev
+                       loopdev=$(setup_loop "${path}" "loop" "/sys/block/loop*")
                        ret="${ret} ${label}=${loopdev}"
                fi
        done
@@ -945,17 +961,19 @@ find_persistence_media ()
        # ${white_list_devices} is non-empty, only devices in it will be
        # scanned.
 
-       local overlays="${1}"
-       local white_listed_devices="${2}"
-       local ret=""
+       local overlays white_listed_devices ret black_listed_devices
+       overlays="${1}"
+       white_listed_devices="${2}"
+       ret=""
 
-       local black_listed_devices="$(what_is_mounted_on ${rootmnt}/lib/live/medium)"
+       black_listed_devices="$(what_is_mounted_on ${rootmnt}/lib/live/medium)"
 
        for dev in $(storage_devices "${black_listed_devices}" "${white_listed_devices}")
        do
-               local result=""
+               local result luks_device
+               result=""
 
-               local luks_device=""
+               luks_device=""
                # Check if it's a luks device; we'll have to open the device
                # in order to probe any filesystem it contains, like we do
                # below. activate_custom_mounts() also depends on that any luks
@@ -1131,11 +1149,12 @@ link_files ()
        # is non-empty, remove mask from all source paths when
        # creating links (will be necessary if we change root, which
        # live-boot normally does (into $rootmnt)).
+       local src_dir dest_dir src_mask
 
        # remove multiple /:s and ensure ending on /
-       local src_dir="$(trim_path ${1})/"
-       local dest_dir="$(trim_path ${2})/"
-       local src_mask="${3}"
+       src_dir="$(trim_path ${1})/"
+       dest_dir="$(trim_path ${2})/"
+       src_mask="${3}"
 
        # This check can only trigger on the inital, non-recursive call since
        # we create the destination before recursive calls
@@ -1148,7 +1167,8 @@ link_files ()
        find "${src_dir}" -mindepth 1 -maxdepth 1 | \
        while read src
        do
-               local dest="${dest_dir}$(basename "${src}")"
+               local dest final_src
+               dest="${dest_dir}$(basename "${src}")"
                if [ -d "${src}" ]
                then
                        if [ -z "$(ls -A "${src}")" ]
@@ -1163,7 +1183,7 @@ link_files ()
                        fi
                        link_files "${src}" "${dest}" "${src_mask}"
                else
-                       local final_src=${src}
+                       final_src=${src}
                        if [ -n "${src_mask}" ]
                        then
                                final_src="$(echo ${final_src} | sed "s|^${src_mask}||")"
@@ -1177,10 +1197,11 @@ link_files ()
 
 do_union ()
 {
-       local unionmountpoint="${1}"    # directory where the union is mounted
-       local unionrw="${2}"            # branch where the union changes are stored
-       local unionro1="${3}"           # first underlying read-only branch (optional)
-       local unionro2="${4}"           # second underlying read-only branch (optional)
+       local unionmountpoint unionrw unionro1 unionro2
+       unionmountpoint="${1}"  # directory where the union is mounted
+       unionrw="${2}"          # branch where the union changes are stored
+       unionro1="${3}"         # first underlying read-only branch (optional)
+       unionro2="${4}"         # second underlying read-only branch (optional)
 
        case "${UNIONTYPE}" in
                aufs)
@@ -1245,12 +1266,13 @@ get_custom_mounts ()
        # Side-effect: leaves $devices with persistence.conf mounted in ${rootmnt}/lib/live/mount/persistence
        # Side-effect: prints info to file $custom_mounts
 
-       local custom_mounts=${1}
+       local custom_mounts devices bindings links
+       custom_mounts=${1}
        shift
-       local devices=${@}
+       devices=${@}
 
-       local bindings="/tmp/bindings.list"
-       local links="/tmp/links.list"
+       bindings="/tmp/bindings.list"
+       links="/tmp/links.list"
        rm -rf ${bindings} ${links} 2> /dev/null
 
        for device in ${devices}
@@ -1260,14 +1282,14 @@ get_custom_mounts ()
                        continue
                fi
 
-               local device_name="$(basename ${device})"
-               local backing=$(mount_persistence_media ${device})
+               local device_name backing include_list
+               device_name="$(basename ${device})"
+               backing=$(mount_persistence_media ${device})
                if [ -z "${backing}" ]
                then
                        continue
                fi
 
-               local include_list
                if [ -r "${backing}/${persistence_list}" ]
                then
                        include_list="${backing}/${persistence_list}"
@@ -1297,8 +1319,9 @@ get_custom_mounts ()
                                continue
                        fi
 
-                       local opt_source=""
-                       local opt_link=""
+                       local opt_source opt_link source full_source full_dest
+                       opt_source=""
+                       opt_link=""
                        for opt in $(echo ${options} | tr ',' ' ');
                        do
                                case "${opt}" in
@@ -1317,7 +1340,7 @@ get_custom_mounts ()
                                esac
                        done
 
-                       local source="${dir}"
+                       source="${dir}"
                        if [ -n "${opt_source}" ]
                        then
                                if echo ${opt_source} | grep -q -e "^/" -e "^\(.*/\)\?\.\.\?\(/.*\)\?$" && [ "${opt_source}" != "." ]
@@ -1329,8 +1352,8 @@ get_custom_mounts ()
                                fi
                        fi
 
-                       local full_source="$(trim_path ${backing}/${source})"
-                       local full_dest="$(trim_path ${rootmnt}/${dir})"
+                       full_source="$(trim_path ${backing}/${source})"
+                       full_dest="$(trim_path ${rootmnt}/${dir})"
                        if [ -n "${opt_link}" ]
                        then
                                echo "${device} ${full_source} ${full_dest} ${options}" >> ${links}
@@ -1351,8 +1374,9 @@ get_custom_mounts ()
 
        # We need to make sure that no two custom mounts have the same sources
        # or are nested; if that is the case, too much weird stuff can happen.
-       local prev_source="impossible source" # first iteration must not match
-       local prev_dest=""
+       local prev_source prev_dest
+       prev_source="impossible source" # first iteration must not match
+       prev_dest=""
        # This sort will ensure that a source /a comes right before a source
        # /a/b so we only need to look at the previous source
        sort -k2 -b ${custom_mounts} |
@@ -1369,14 +1393,16 @@ get_custom_mounts ()
 
 activate_custom_mounts ()
 {
-       local custom_mounts="${1}" # the ouput from get_custom_mounts()
-       local used_devices=""
+       local custom_mounts used_devices
+       custom_mounts="${1}" # the ouput from get_custom_mounts()
+       used_devices=""
 
        while read device source dest options # < ${custom_mounts}
        do
-               local opt_bind="true"
-               local opt_link=""
-               local opt_union=""
+               local opt_bind opt_link opt_union
+               opt_bind="true"
+               opt_link=""
+               opt_union=""
                for opt in $(echo ${options} | tr ',' ' ');
                do
                        case "${opt}" in
@@ -1457,7 +1483,8 @@ activate_custom_mounts ()
 
                # XXX: If CONFIG_AUFS_ROBR is added to the Debian kernel we can
                # ignore the loop below and set rootfs_dest_backing=$dest
-               local rootfs_dest_backing=""
+               local rootfs_dest_backing
+               rootfs_dest_backing=""
                if [ -n "${opt_link}"]
                then
                        for d in ${rootmnt}/lib/live/mount/rootfs/*
@@ -1477,13 +1504,14 @@ activate_custom_mounts ()
                        done
                fi
 
+               local cow_dir links_source
                if [ -n "${opt_link}" ] && [ -z "${PERSISTENCE_READONLY}" ]
                then
                        link_files ${source} ${dest} ${rootmnt}
                elif [ -n "${opt_link}" ] && [ -n "${PERSISTENCE_READONLY}" ]
                then
                        mkdir -p ${rootmnt}/lib/live/mount/persistence
-                       local links_source=$(mktemp -d ${rootmnt}/lib/live/mount/persistence/links-source-XXXXXX)
+                       links_source=$(mktemp -d ${rootmnt}/lib/live/mount/persistence/links-source-XXXXXX)
                        chown_ref ${source} ${links_source}
                        chmod_ref ${source} ${links_source}
                        # We put the cow dir in the below strange place to
@@ -1491,7 +1519,7 @@ activate_custom_mounts ()
                        # has its own directory and isn't nested with some
                        # other custom mount (if so that mount's files would
                        # be linked, causing breakage.
-                       local cow_dir="${rootmnt}/lib/live/mount/overlay/lib/live/mount/persistence/$(basename ${links_source})"
+                       cow_dir="${rootmnt}/lib/live/mount/overlay/lib/live/mount/persistence/$(basename ${links_source})"
                        mkdir -p ${cow_dir}
                        chown_ref "${source}" "${cow_dir}"
                        chmod_ref "${source}" "${cow_dir}"
@@ -1508,7 +1536,7 @@ activate_custom_mounts ()
                        # bind-mount and union mount are handled the same
                        # in read-only mode, but note that rootfs_dest_backing
                        # is non-empty (and necessary) only for unions
-                       local cow_dir="${rootmnt}/lib/live/mount/overlay/${dest}"
+                       cow_dir="${rootmnt}/lib/live/mount/overlay/${dest}"
                        if [ -e "${cow_dir}" ] && [ -z "${opt_link}" ]
                        then
                                # If an earlier custom mount has files here
@@ -1536,22 +1564,23 @@ activate_custom_mounts ()
 
 fix_backwards_compatibility ()
 {
-       local device=${1}
-       local dir=${2}
-       local opt=${3}
+       local device dir opt backing include_list
+       device=${1}
+       dir=${2}
+       opt=${3}
 
        if [ -n "${PERSISTENCE_READONLY}" ]
        then
                return
        fi
 
-       local backing="$(mount_persistence_media ${device})"
+       backing="$(mount_persistence_media ${device})"
        if [ -z "${backing}" ]
        then
                return
        fi
 
-       local include_list="${backing}/${persistence_list}"
+       include_list="${backing}/${persistence_list}"
        if [ ! -r "${include_list}" ]
        then
                echo "# persistence backwards compatibility:

diff --git a/scripts/boot/9990-mount-http.sh b/scripts/boot/9990-mount-http.sh
index 1b718c0..b557404 100755 (executable)
--- a/scripts/boot/9990-mount-http.sh
+++ b/scripts/boot/9990-mount-http.sh
@@ -8,8 +8,9 @@ do_httpmount ()
 
        for webfile in HTTPFS FTPFS FETCH
        do
-               local url="$(eval echo \"\$\{${webfile}\}\")"
-               local extension="$(echo "${url}" | sed 's/\(.*\)\.\(.*\)/\2/')"
+               local url extension dest
+               url="$(eval echo \"\$\{${webfile}\}\")"
+               extension="$(echo "${url}" | sed 's/\(.*\)\.\(.*\)/\2/')"
 
                if [ -n "$url" ]
                then
@@ -20,7 +21,7 @@ do_httpmount ()
                                                mkdir -p "${alt_mountpoint}"
                                                dest="${alt_mountpoint}"
                                        else
-                                               local dest="${mountpoint}/${LIVE_MEDIA_PATH}"
+                                               dest="${mountpoint}/${LIVE_MEDIA_PATH}"
                                                mount -t ramfs ram "${mountpoint}"
                                                mkdir -p "${dest}"
                                        fi

diff --git a/scripts/boot/9990-mount-iscsi.sh b/scripts/boot/9990-mount-iscsi.sh
index fd29d91..6ce9851 100755 (executable)
--- a/scripts/boot/9990-mount-iscsi.sh
+++ b/scripts/boot/9990-mount-iscsi.sh
@@ -7,7 +7,8 @@ do_iscsi()
        do_netsetup
        #modprobe ib_iser
        modprobe iscsi_tcp
-       local debugopt=""
+       local debugopt
+       debugopt=""
        [ "${DEBUG}" = "true" ] && debugopt="-d 8"
        #FIXME this name is supposed to be unique - some date + ifconfig hash?
        ISCSI_INITIATORNAME="iqn.1993-08.org.debian.live:01:$(echo "${HWADDR}" | sed -e s/://g)"
@@ -21,12 +22,14 @@ do_iscsi()
        then
                panic "Failed to log into iscsi target"
        fi
-       local host="$(ls -d /sys/class/scsi_host/host*/device/iscsi_host:host* \
-                           /sys/class/scsi_host/host*/device/iscsi_host/host* | sed -e 's:/device.*::' -e 's:.*host::')"
+       local host
+       host="$(ls -d /sys/class/scsi_host/host*/device/iscsi_host:host* \
+                     /sys/class/scsi_host/host*/device/iscsi_host/host* | sed -e 's:/device.*::' -e 's:.*host::')"
        if [ -n "${host}" ]
        then
-               local devices=""
-               local i=0
+               local devices i
+               devices=""
+               i=0
                while [ -z "${devices}" -a $i -lt 60 ]
                do
                        sleep 1

diff --git a/scripts/boot/9990-overlay.sh b/scripts/boot/9990-overlay.sh
index 96c3331..80b5726 100755 (executable)
--- a/scripts/boot/9990-overlay.sh
+++ b/scripts/boot/9990-overlay.sh
@@ -189,7 +189,8 @@ setup_unionfs ()
                        done
                fi
 
-               local whitelistdev=""
+               local whitelistdev
+               whitelistdev=""
                if [ -n "${PERSISTENCE_MEDIA}" ]
                then
                        case "${PERSISTENCE_MEDIA}" in
@@ -212,7 +213,8 @@ setup_unionfs ()
                        overlays="${old_root_overlay_label} ${old_home_overlay_label} ${custom_overlay_label}"
                fi
 
-               local overlay_devices=""
+               local overlay_devices
+               overlay_devices=""
                if [ "${whitelistdev}" != "ignore_all_devices" ]
                then
                        for media in $(find_persistence_media "${overlays}" "${whitelistdev}")
@@ -406,7 +408,8 @@ setup_unionfs ()
        # Adding custom persistence
        if [ -n "${PERSISTENCE}" ] && [ -z "${NOPERSISTENCE}" ]
        then
-               local custom_mounts="/tmp/custom_mounts.list"
+               local custom_mounts
+               custom_mounts="/tmp/custom_mounts.list"
                rm -rf ${custom_mounts} 2> /dev/null
 
                # Gather information about custom mounts from devies detected as overlays
@@ -415,7 +418,8 @@ setup_unionfs ()
                [ -n "${DEBUG}" ] && cp ${custom_mounts} "/lib/live/mount/persistence"
 
                # Now we do the actual mounting (and symlinking)
-               local used_overlays=""
+               local used_overlays
+               used_overlays=""
                used_overlays=$(activate_custom_mounts ${custom_mounts})
                rm ${custom_mounts}