[systemd-devel] [PATCH] kernel-install: Clean up

[Mantas Mikulėnas]

- Consistent use of $VAR vs ${VAR}
- Consistent use of && vs 'if'
- Add error checking to some places
- Consistent error messages ("Can't" vs "Cannot", etc.)
- Function declarations at the top
- Miscellaneous adjustments
---
 src/kernel-install/kernel-install | 137 +++++++++++++++++++++-----------------
 1 file changed, 75 insertions(+), 62 deletions(-)

diff --git a/src/kernel-install/kernel-install b/src/kernel-install/kernel-install
index 9f3a523..be4a8e2 100644
--- a/src/kernel-install/kernel-install
+++ b/src/kernel-install/kernel-install
@@ -19,23 +19,62 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with systemd; If not, see <http://www.gnu.org/licenses/>.
 
+usage()
+{
+    echo "Usage:" >&2
+    echo "        $0 add <kernel-version> <kernel-image>" >&2
+    echo "        $0 remove <kernel-version> <kernel-image>" >&2
+}
+
+dropindirs_sort()
+{
+    local suffix=$1; shift
+    local -a files
+    local f d i
+
+    readarray -t files < <(
+        for d in "$@"; do
+            for i in "$d/"*"$suffix"; do
+                if [[ -e "$i" ]]; then
+                    echo "${i##*/}"
+                fi
+            done
+        done | sort -Vu
+    )
+
+    for f in "${files[@]}"; do
+        for d in "$@"; do
+            if [[ -e "$d/$f" ]]; then
+                echo "$d/$f"
+                continue 2
+            fi
+        done
+    done
+}
+
 export LC_COLLATE=C
 
 COMMAND="$1"
 KERNEL_VERSION="$2"
 KERNEL_IMAGE="$3"
 
-[[ -f /etc/os-release ]] && . /etc/os-release
+if [[ -f /etc/os-release ]]; then
+    . /etc/os-release
+fi
+
 if ! [[ $ID ]]; then
-    echo "Can't determine the name of your distribution. Please create /etc/os-release." >&2
-    echo "See man:os-release(5)" >&2
+    echo "Could not determine the distribution name from /etc/os-release." >&2
+    echo "Please specify ID=... in /etc/os-release. See man:os-release(5)" >&2
     exit 1
 fi
 
-[[ -f /etc/machine-id ]] && read MACHINE_ID < /etc/machine-id
+if [[ -f /etc/machine-id ]]; then
+    read MACHINE_ID < /etc/machine-id
+fi
+
 if ! [[ $MACHINE_ID ]]; then
-    echo "Can't determine your machine id. Please create /etc/machine-id!" >&2
-    echo "See man:machine-id(5)" >&2
+    echo "Could not determine your machine ID from /etc/machine-id." >&2
+    echo "Please run 'systemd-machine-id-setup' as root. See man:machine-id(5)" >&2
     exit 1
 fi
 
@@ -43,97 +82,71 @@ if [[ -f /etc/kernel/cmdline ]]; then
     readarray -t BOOT_OPTIONS < /etc/kernel/cmdline
 fi
 
-if ! [[ "${BOOT_OPTIONS[@]}" ]]; then
+if ! [[ ${BOOT_OPTIONS[*]} ]]; then
     readarray -t BOOT_OPTIONS < /proc/cmdline
 fi
 
 if ! [[ $BOOT_OPTIONS ]]; then
-    echo "Can't determine the kernel command line parameters." >&2
+    echo "Could not determine the kernel command line parameters." >&2
     echo "Please specify the kernel command line in /etc/kernel/cmdline!" >&2
     exit 1
 fi
 
-usage()
-{
-    {
-        echo "Usage:"
-        echo "        $0 add <kernel-version> <kernel-image>"
-        echo "        $0 remove <kernel-version> <kernel-image>"
-    } >&2
-}
-
-if ! ( [[ $COMMAND ]] && [[ $KERNEL_VERSION ]] ); then
+if [[ ! $COMMAND ]] || [[ ! $KERNEL_VERSION ]]; then
     usage
     exit 1
 fi
 
-BOOT_DIR="/${MACHINE_ID}/${KERNEL_VERSION}"
-BOOT_DIR_ABS="/boot${BOOT_DIR}"
-LOADER_ENTRY="/boot/loader/entries/${MACHINE_ID}-${KERNEL_VERSION}.conf"
+BOOT_DIR="/$MACHINE_ID/$KERNEL_VERSION"
+BOOT_DIR_ABS="/boot$BOOT_DIR"
+LOADER_ENTRY="/boot/loader/entries/$MACHINE_ID-$KERNEL_VERSION.conf"
 ret=0
 
-dropindirs_sort()
-{
-    suffix=$1; shift
-    readarray -t files< <(
-        for d in "$@"; do
-            for i in "${d}/"*${suffix}; do
-                [[ -e $i ]] && echo ${i##*/}
-            done
-        done | sort -Vu
-    )
-
-    for f in "${files[@]}"; do
-        for d in "$@"; do
-            if [[ -e "$d/$f" ]]; then
-                echo "$d/$f"
-                continue 2
-            fi
-        done
-    done
-}
-
 readarray -t PLUGINS < <(
     dropindirs_sort ".install" \
         "/etc/kernel/install.d" \
         "/usr/lib/kernel/install.d"
 )
 
-case "$COMMAND" in
+case $COMMAND in
     add)
-        if [[ -z $KERNEL_IMAGE ]]; then
+        if [[ ! $KERNEL_IMAGE ]]; then
             usage
             exit 1
         fi
-        mkdir -p "$BOOT_DIR_ABS" || exit 1
+
+        mkdir -p "$BOOT_DIR_ABS" || {
+            echo "Could not create boot directory '$BOOT_DIR_ABS'." >&2
+            exit 1
+        }
 
         for f in "${PLUGINS[@]}"; do
             [[ -x $f ]] && "$f" add "$KERNEL_VERSION" "$BOOT_DIR_ABS"
             ((ret+=$?))
         done
 
-        if ! cp --preserve "$KERNEL_IMAGE" "$BOOT_DIR_ABS"/linux; then
-            echo "Can't copy '$KERNEL_IMAGE to '$BOOT_DIR_ABS/linux'!" >&2
-        fi
+        cp --preserve "$KERNEL_IMAGE" "$BOOT_DIR_ABS/linux" || {
+            echo "Could not copy '$KERNEL_IMAGE to '$BOOT_DIR_ABS/linux'." >&2
+            exit 1
+        }
 
-        [[ -d /boot/loader/entries ]] || mkdir -p /boot/loader/entries
+        mkdir -p "${LOADER_ENTRY%/*}" || {
+            echo "Could not create loader entry directory '${LOADER_ENTRY%/*}'." >&2
+            exit 1
+        }
 
         {
             echo "title      $PRETTY_NAME"
             echo "version    $KERNEL_VERSION"
             echo "machine-id $MACHINE_ID"
-            echo "options    ${BOOT_OPTIONS[@]}"
+            echo "options    ${BOOT_OPTIONS[*]}"
             echo "linux      $BOOT_DIR/linux"
-            [[ -f "${BOOT_DIR_ABS}"/initrd ]] && \
+            [[ -f $BOOT_DIR_ABS/initrd ]] && \
                 echo "initrd     $BOOT_DIR/initrd"
-            :
-        } > $LOADER_ENTRY
-
-        ((ret+=$?))
-
-        if ! [[ -f $LOADER_ENTRY ]]; then
-            echo "Could not create '$LOADER_ENTRY'!" >&2
-        fi
+        } > "$LOADER_ENTRY" || {
+            echo "Could not create loader entry '$LOADER_ENTRY'." >&2
+            exit 1
+        }
         ;;
 
     remove)
@@ -142,13 +155,13 @@ case "$COMMAND" in
             ((ret+=$?))
         done
 
-        rm -fr "$BOOT_DIR_ABS"
-        rm -f "$LOADER_ENTRY"
+        rm -rf "$LOADER_ENTRY" "$BOOT_DIR_ABS"
         ;;
 
     *)
         usage
-        ret=1;;
+        exit 1
+        ;;
 esac
 
 ((ret+=$?))
-- 
1.8.2.1.888.g0235b3b