From 79497e00dcc912bc42ef920b299fed081cf07359 Mon Sep 17 00:00:00 2001 From: Graham Percival Date: Sun, 6 Aug 2023 17:17:36 -0700 Subject: [PATCH] scripts: always use double quotes and curly braces Command line: shellcheck -s sh -i SC2248,SC2250 -o quote-safe-variables,require-variable-braces $(git ls-files "*.sh") (There will be two complaints about wanting to double-quote ${CFLAGS_C99}, but it's constructing a command-line, so ignore those warnings.) Explanation: - Prefer double quoting even when variables don't contain special characters. https://www.shellcheck.net/wiki/SC2248 - Prefer putting braces around variable references even when not strictly required. https://github.com/koalaman/shellcheck/wiki/SC2250 Reported by: shellcheck 0.9.0 --- get-version.sh | 2 +- libcperciva/POSIX/posix-cflags.sh | 42 +++++++++++----------- libcperciva/POSIX/posix-l.sh | 6 ++-- libcperciva/cpusupport/Build/cpusupport.sh | 10 +++--- release-tools/create-sign-tarball.sh | 4 +-- release-tools/mktarball.sh | 4 +-- tests/shared_test_functions.sh | 12 +++---- tests/shared_valgrind_functions.sh | 12 +++---- 8 files changed, 46 insertions(+), 46 deletions(-) diff --git a/get-version.sh b/get-version.sh index 3c42f5be..51d3db42 100644 --- a/get-version.sh +++ b/get-version.sh @@ -21,7 +21,7 @@ if git rev-parse 2>/dev/null; then # Get a version string from the latest git tag. if version_git=$( git describe --tags --match '[[:digit:]].*' ) \ 2>/dev/null ; then - version_decapitated=$( echo ${version} | sed "s/-head//" ) + version_decapitated=$( echo "${version}" | sed "s/-head//" ) # Check that the beginning of this tag matches the version. case ${version_git} in "${version_decapitated}"*) diff --git a/libcperciva/POSIX/posix-cflags.sh b/libcperciva/POSIX/posix-cflags.sh index 5810da3a..957844a5 100644 --- a/libcperciva/POSIX/posix-cflags.sh +++ b/libcperciva/POSIX/posix-cflags.sh @@ -8,7 +8,7 @@ if [ -z "${CC}" ]; then exit 1 fi if ! [ "${PATH}" = "$1" ]; then - echo "WARNING: POSIX violation: $SHELL's command -p resets \$PATH" 1>&2 + echo "WARNING: POSIX violation: ${SHELL}'s command -p resets \$PATH" 1>&2 PATH=$1 fi @@ -16,7 +16,7 @@ fi D=$(dirname "$0") # Check if we can compile & run a binary. -if ! ${CC} ${CFLAGS} "$D/posix-trivial.c" 2>/dev/null; then +if ! ${CC} ${CFLAGS} "${D}/posix-trivial.c" 2>/dev/null; then echo "WARNING: failed to compile posix-trivial.c" 1>&2 else # If the user hasn't disabled runtime checks... @@ -31,28 +31,28 @@ else fi FIRST=YES -if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L "$D/posix-msg_nosignal.c" 2>/dev/null; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO +if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L "${D}/posix-msg_nosignal.c" 2>/dev/null; then + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-DPOSIXFAIL_MSG_NOSIGNAL" echo "WARNING: POSIX violation: not defining MSG_NOSIGNAL" 1>&2 fi -if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L "$D/posix-clock_realtime.c" 2>/dev/null; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO +if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L "${D}/posix-clock_realtime.c" 2>/dev/null; then + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-DPOSIXFAIL_CLOCK_REALTIME" echo "WARNING: POSIX violation: not defining CLOCK_REALTIME" 1>&2 fi -if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L -D_XOPEN_SOURCE=700 "$D/posix-inet-addrstrlen.c" 2>/dev/null; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO +if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L -D_XOPEN_SOURCE=700 "${D}/posix-inet-addrstrlen.c" 2>/dev/null; then + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-DPOSIXFAIL_INET_ADDRSTRLEN" echo "WARNING: POSIX violation: not defining INET_ADDRSTRLEN" 1>&2 fi -if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L -D_XOPEN_SOURCE=700 "$D/posix-inet6-addrstrlen.c" 2>/dev/null; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO +if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L -D_XOPEN_SOURCE=700 "${D}/posix-inet6-addrstrlen.c" 2>/dev/null; then + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-DPOSIXFAIL_INET6_ADDRSTRLEN" echo "WARNING: POSIX violation: not defining INET6_ADDRSTRLEN" 1>&2 fi -if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L "$D/posix-clock_gettime.c" 2>/dev/null; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO +if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L "${D}/posix-clock_gettime.c" 2>/dev/null; then + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-DPOSIXFAIL_CLOCK_GETTIME" echo "WARNING: POSIX violation: not declaring clock_gettime()" 1>&2 elif [ "${DISABLE_POSIX_RUNTIME_CHECKS:-0}" -ne "0" ]; then @@ -68,30 +68,30 @@ else # calling process. The "( ./x 2>y ) 2>y" captures both types of error # message. if ! ( ./a.out 2>/dev/null ) 2>/dev/null ; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-DPOSIXFAIL_CLOCK_GETTIME" echo "WARNING: POSIX violation: clock_gettime() is not linkable" 1>&2 fi fi -if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L -D_XOPEN_SOURCE=700 "$D/posix-stat-st_mtim.c" 2>/dev/null; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO +if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L -D_XOPEN_SOURCE=700 "${D}/posix-stat-st_mtim.c" 2>/dev/null; then + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-DPOSIXFAIL_STAT_ST_MTIM" echo "WARNING: POSIX violation: struct stat does not contain st_mtim" 1>&2 fi CFLAGS_C99="" -if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L "$D/posix-restrict.c" 2>/dev/null; then +if ! ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L "${D}/posix-restrict.c" 2>/dev/null; then echo "WARNING: POSIX violation: ${CC} does not accept the 'restrict' keyword" 1>&2 - if ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L -std=c99 "$D/posix-restrict.c" 2>/dev/null; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO + if ${CC} ${CFLAGS} -D_POSIX_C_SOURCE=200809L -std=c99 "${D}/posix-restrict.c" 2>/dev/null; then + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-std=c99" CFLAGS_C99="-std=c99" fi fi -if ! ${CC} ${CFLAGS} ${CFLAGS_C99} -D_POSIX_C_SOURCE=200809L -DARGNAME="" "$D/posix-abstract-declarator.c" 2>/dev/null; then +if ! ${CC} ${CFLAGS} ${CFLAGS_C99} -D_POSIX_C_SOURCE=200809L -DARGNAME="" "${D}/posix-abstract-declarator.c" 2>/dev/null; then echo "WARNING: POSIX violation: ${CC} does not accept qualifiers in an abstract declarator" 1>&2 # Test compile with -DPOSIXFAIL_ABSTRACT_DECLARATOR - if ${CC} ${CFLAGS} ${CFLAGS_C99} -D_POSIX_C_SOURCE=200809L -DPOSIXFAIL_ABSTRACT_DECLARATOR "$D/posix-abstract-declarator.c" 2>/dev/null; then - [ ${FIRST} = "NO" ] && printf " "; FIRST=NO + if ${CC} ${CFLAGS} ${CFLAGS_C99} -D_POSIX_C_SOURCE=200809L -DPOSIXFAIL_ABSTRACT_DECLARATOR "${D}/posix-abstract-declarator.c" 2>/dev/null; then + [ "${FIRST}" = "NO" ] && printf " "; FIRST=NO printf %s "-DPOSIXFAIL_ABSTRACT_DECLARATOR" fi fi diff --git a/libcperciva/POSIX/posix-l.sh b/libcperciva/POSIX/posix-l.sh index ef61a745..c8f90d35 100644 --- a/libcperciva/POSIX/posix-l.sh +++ b/libcperciva/POSIX/posix-l.sh @@ -8,7 +8,7 @@ if [ -z "${CC}" ]; then exit 1 fi if ! [ "${PATH}" = "$1" ]; then - echo "WARNING: POSIX violation: $SHELL's command -p resets \$PATH" 1>&2 + echo "WARNING: POSIX violation: ${SHELL}'s command -p resets \$PATH" 1>&2 PATH=$1 fi @@ -17,8 +17,8 @@ D=$(dirname "$0") FIRST=YES for LIB in rt xnet; do - if ${CC} ${CFLAGS} -l${LIB} "$D/posix-trivial.c" 2>/dev/null; then - if [ ${FIRST} = "NO" ]; then + if ${CC} ${CFLAGS} -l"${LIB}" "${D}/posix-trivial.c" 2>/dev/null; then + if [ "${FIRST}" = "NO" ]; then printf " "; fi printf "%s" "-l${LIB}"; diff --git a/libcperciva/cpusupport/Build/cpusupport.sh b/libcperciva/cpusupport/Build/cpusupport.sh index c179178a..f977894b 100755 --- a/libcperciva/cpusupport/Build/cpusupport.sh +++ b/libcperciva/cpusupport/Build/cpusupport.sh @@ -1,7 +1,7 @@ # Should be sourced by `command -p sh path/to/cpusupport.sh "$PATH"` from # within a Makefile. if ! [ "${PATH}" = "$1" ]; then - echo "WARNING: POSIX violation: $SHELL's command -p resets \$PATH" 1>&2 + echo "WARNING: POSIX violation: ${SHELL}'s command -p resets \$PATH" 1>&2 PATH=$1 fi @@ -33,16 +33,16 @@ feature() { # Check if we can compile this feature (and any required arguments). printf "Checking if compiler supports %s %s feature..." \ - "$ARCH" "$FEATURE" 1>&2 + "${ARCH}" "${FEATURE}" 1>&2 for CPU_CFLAGS in "$@"; do if ${CC} ${CPPFLAGS} ${CFLAGS} ${CFLAGS_HARDCODED} \ - ${CPU_CFLAGS} "${feature_filename}" 2>>${outcc}; then + ${CPU_CFLAGS} "${feature_filename}" 2>>"${outcc}"; then rm -f a.out break; fi CPU_CFLAGS=NOTSUPPORTED; done - case $CPU_CFLAGS in + case ${CPU_CFLAGS} in NOTSUPPORTED) echo " no" 1>&2 ;; @@ -51,7 +51,7 @@ feature() { echo "#define CPUSUPPORT_${ARCH}_${FEATURE} 1" ;; *) - echo " yes, via $CPU_CFLAGS" 1>&2 + echo " yes, via ${CPU_CFLAGS}" 1>&2 echo "#define CPUSUPPORT_${ARCH}_${FEATURE} 1" echo "#ifdef cpusupport_dummy" echo "export CFLAGS_${ARCH}_${FEATURE}=\"${CPU_CFLAGS}\"" diff --git a/release-tools/create-sign-tarball.sh b/release-tools/create-sign-tarball.sh index 93fb7f9f..79003e22 100644 --- a/release-tools/create-sign-tarball.sh +++ b/release-tools/create-sign-tarball.sh @@ -5,7 +5,7 @@ SCRYPTVERSION=$1 GNUPG_SIGNING_HOME=$2 # Check for required arguments -if [ -z "$SCRYPTVERSION" ] || [ -z "$GNUPG_SIGNING_HOME" ]; then +if [ -z "${SCRYPTVERSION}" ] || [ -z "${GNUPG_SIGNING_HOME}" ]; then echo "Usage: $0 SCRYPTVERSION GNUPG_SIGNING_HOME" exit 1 fi @@ -26,7 +26,7 @@ PKGSIGS=scrypt-sigs-${SCRYPTVERSION} dir=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P) # Create tarball -sh "${dir}/mktarball.sh" "$SCRYPTVERSION" +sh "${dir}/mktarball.sh" "${SCRYPTVERSION}" # Sign tarball sha256 "${PKGNAME}.tgz" | \ diff --git a/release-tools/mktarball.sh b/release-tools/mktarball.sh index b00b2181..d18fcd99 100644 --- a/release-tools/mktarball.sh +++ b/release-tools/mktarball.sh @@ -1,7 +1,7 @@ #!/bin/sh VERSION=$1 -if [ -z "$VERSION" ]; then +if [ -z "${VERSION}" ]; then echo "Please specify the version number" exit 1 fi @@ -15,7 +15,7 @@ cp Makefile.am .autom4te.cfg "${DESTDIR}" cp Makefile.am "${DESTDIR}/autotools" cp -R lib lib-platform libcperciva libscrypt-kdf m4 tests "${DESTDIR}" # Copy with substitution -sed -e "s/@DATE@/$RELEASEDATE/" < scrypt.1 > "${DESTDIR}/scrypt.1" +sed -e "s/@DATE@/${RELEASEDATE}/" < scrypt.1 > "${DESTDIR}/scrypt.1" sed -e "s/\[m4_esyscmd(\[sh get-version\.sh\])]/${VERSION}/" \ < configure.ac > "${DESTDIR}/configure.ac" cp "${DESTDIR}/configure.ac" "${DESTDIR}/autotools" diff --git a/tests/shared_test_functions.sh b/tests/shared_test_functions.sh index 887a1cd4..ff4fa310 100644 --- a/tests/shared_test_functions.sh +++ b/tests/shared_test_functions.sh @@ -218,7 +218,7 @@ notify_success_or_fail() { # Bail if there's no exitfiles. exitfiles=$(ls "${log_basename}"-*.exit) || true - if [ -z "$exitfiles" ]; then + if [ -z "${exitfiles}" ]; then echo "FAILED" 1>&2 s_retval=1 return @@ -229,7 +229,7 @@ notify_success_or_fail() { skip_exitfiles=0 # Check each exitfile. - for exitfile in $(echo "$exitfiles" | sort); do + for exitfile in $(echo "${exitfiles}" | sort); do ret=$(cat "${exitfile}") total_exitfiles=$(( total_exitfiles + 1 )) if [ "${ret}" -lt 0 ]; then @@ -265,8 +265,8 @@ notify_success_or_fail() { done # Notify about skip or success. - if [ ${skip_exitfiles} -gt 0 ]; then - if [ ${skip_exitfiles} -eq ${total_exitfiles} ]; then + if [ "${skip_exitfiles}" -gt 0 ]; then + if [ "${skip_exitfiles}" -eq "${total_exitfiles}" ]; then echo "SKIP!" 1>&2 else echo "PARTIAL SUCCESS / SKIP!" 1>&2 @@ -334,8 +334,8 @@ run_scenarios() { # want to allow it to echo values to stdout. scenario_runner "${scenario}" retval=$? - if [ ${retval} -gt 0 ]; then - exit ${retval} + if [ "${retval}" -gt 0 ]; then + exit "${retval}" fi done } diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh index d24f6887..e4e9033b 100644 --- a/tests/shared_valgrind_functions.sh +++ b/tests/shared_valgrind_functions.sh @@ -51,7 +51,7 @@ valgrind_prepare_directory() { fi # Bail if we don't want valgrind at all. - if [ "$USE_VALGRIND" -eq 0 ]; then + if [ "${USE_VALGRIND}" -eq 0 ]; then return fi @@ -72,7 +72,7 @@ valgrind_prepare_directory() { # Return a $USE_VALGRIND variable defined; if it was previously defined and # was greater than 0, then check that valgrind is available in the $PATH. valgrind_check_optional() { - if [ "$USE_VALGRIND" -gt 0 ]; then + if [ "${USE_VALGRIND}" -gt 0 ]; then # Look for valgrind in $PATH. if ! command -v valgrind >/dev/null 2>&1; then printf "valgrind not found\n" 1>&2 @@ -120,8 +120,8 @@ valgrind_process_suppression_file() { # Skip "${filename}00" because that doesn't contain a suppression. i=1 - while [ "$i" -le "${num_segments}" ]; do - segfilename="$(printf "%s%02i" "${filename}" "$i")" + while [ "${i}" -le "${num_segments}" ]; do + segfilename="$(printf "%s%02i" "${filename}" "${i}")" # Find last relevant line. lastline="$(grep -n "}" "${segfilename}" | cut -f1 -d:)" @@ -149,7 +149,7 @@ valgrind_process_suppression_file() { # Only keep the beginning of each suppression. lastline="$((lastline - 1))" - head -n "$lastline" "${segfilename}" >> \ + head -n "${lastline}" "${segfilename}" >> \ "${valgrind_suppressions}" printf "}\n" >> "${valgrind_suppressions}" @@ -167,7 +167,7 @@ valgrind_ensure_suppression() { potential_memleaks_binary=$1 # Quit if we're not using valgrind. - if [ ! "$USE_VALGRIND" -gt 0 ]; then + if [ ! "${USE_VALGRIND}" -gt 0 ]; then return fi;