From 2b7b0387730e5a4f2328ff90d8b4f02bb808cb40 Mon Sep 17 00:00:00 2001 From: Graham Percival Date: Wed, 11 Oct 2023 19:44:13 -0700 Subject: [PATCH 1/4] tests: shorten some of the names in the public API A later commit will switch to using "local" variables (indicated by prepending the variable name with the function name), but those would be extremely long with the previous function names. Shortening them will result in variables which are merely very long. Changes to the overall API: s/setup_check_variables/setup_check/g Changes to the valgrind API: s/valgrind_setup_cmd/valgrind_setup/g s/valgrind_check_basenames/valgrind_check/g --- tests/01-known-values.sh | 4 ++-- tests/02-decrypt-reference-file.sh | 12 ++++++------ tests/03-encrypt-decrypt-file.sh | 8 ++++---- tests/04-force-resources.sh | 10 +++++----- tests/05-system-scrypt-encrypt-decrypt.sh | 14 +++++++------- tests/06-decrypt-fail.sh | 6 +++--- tests/07-passphrase-env.sh | 16 ++++++++-------- tests/08-passphrase-file.sh | 16 ++++++++-------- tests/09-explicit-params.sh | 18 +++++++++--------- tests/shared_test_functions.sh | 10 +++++----- tests/shared_valgrind_functions.sh | 12 ++++++------ 11 files changed, 63 insertions(+), 63 deletions(-) diff --git a/tests/01-known-values.sh b/tests/01-known-values.sh index 68848889..4b3b21b9 100644 --- a/tests/01-known-values.sh +++ b/tests/01-known-values.sh @@ -11,7 +11,7 @@ reference_small="${scriptdir}/verify-strings/test_scrypt_small.good" ### Actual command scenario_cmd() { # Run the binary which tests known input/output strings. - setup_check_variables "test_scrypt" + setup_check "test_scrypt" ( ${c_valgrind_cmd} "${bindir}/tests/verify-strings/test_scrypt" \ "${SMALLMEM:-0}" 1> "${test_output}" @@ -19,7 +19,7 @@ scenario_cmd() { ) # The generated values should match the known good values. - setup_check_variables "test_scrypt output against reference" + setup_check "test_scrypt output against reference" if [ "${SMALLMEM:-0}" -gt "0" ]; then cmp -s "${test_output}" "${reference_small}" else diff --git a/tests/02-decrypt-reference-file.sh b/tests/02-decrypt-reference-file.sh index 4deb0825..0f2849d6 100644 --- a/tests/02-decrypt-reference-file.sh +++ b/tests/02-decrypt-reference-file.sh @@ -11,7 +11,7 @@ decrypted_badpass_log="${s_basename}-decrypt-badpass.log" scenario_cmd() { # Decrypt a reference file. - setup_check_variables "scrypt dec" + setup_check "scrypt dec" ( echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ dec -P "${encrypted_reference_file}" \ @@ -21,18 +21,18 @@ scenario_cmd() { ) # The decrypted reference file should match the reference. - setup_check_variables "scrypt dec output against reference" + setup_check "scrypt dec output against reference" cmp -s "${decrypted_reference_file}" "${reference_file}" echo $? > "${c_exitfile}" # We should not have any output on stderr. - setup_check_variables "scrypt dec no stderr" + setup_check "scrypt dec no stderr" test -s "${decrypted_reference_file_stderr}" expected_exitcode 1 $? > "${c_exitfile}" # Attempt to decrypt the reference file with an incorrect passphrase. # We want this command to fail with 1. - setup_check_variables "scrypt dec bad passphrase" + setup_check "scrypt dec bad passphrase" ( echo "bad-pass" | ${c_valgrind_cmd} "${bindir}/scrypt" \ dec -P "${encrypted_reference_file}" \ @@ -42,7 +42,7 @@ scenario_cmd() { ) # We should have received an error message. - setup_check_variables "scrypt dec bad passphrase error" + setup_check "scrypt dec bad passphrase error" if grep -q "scrypt: Passphrase is incorrect" \ "${decrypted_badpass_log}"; then echo "0" @@ -51,7 +51,7 @@ scenario_cmd() { fi > "${c_exitfile}" # We should not have created a file. - setup_check_variables "scrypt dec bad passphrase no file" + setup_check "scrypt dec bad passphrase no file" if [ -e "${decrypted_badpass_file}" ]; then echo "1" else diff --git a/tests/03-encrypt-decrypt-file.sh b/tests/03-encrypt-decrypt-file.sh index c1430ecf..0454f8e4 100644 --- a/tests/03-encrypt-decrypt-file.sh +++ b/tests/03-encrypt-decrypt-file.sh @@ -8,7 +8,7 @@ decrypted_file="${s_basename}-attempt.txt" scenario_cmd() { # Encrypt a file. Use --passphrase dev:stdin-once instead of -P. - setup_check_variables "scrypt enc" + setup_check "scrypt enc" ( echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ enc --passphrase dev:stdin-once -t 1 \ @@ -20,12 +20,12 @@ scenario_cmd() { # We cannot check against the "reference" encrypted file, because # encrypted files include random salt. If successful, don't delete # ${encrypted_file} yet; we need it for the next test. - setup_check_variables "scrypt enc random salt" + setup_check "scrypt enc random salt" cmp -s "${encrypted_file}" "${reference_file}" expected_exitcode 1 $? > "${c_exitfile}" # Decrypt the file we just encrypted. - setup_check_variables "scrypt enc decrypt" + setup_check "scrypt enc decrypt" ( echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ dec -P "${encrypted_file}" "${decrypted_file}" @@ -33,7 +33,7 @@ scenario_cmd() { ) # The decrypted file should match the reference. - setup_check_variables "scrypt enc decrypt output against reference" + setup_check "scrypt enc decrypt output against reference" cmp -s "${decrypted_file}" "${reference_file}" echo $? > "${c_exitfile}" } diff --git a/tests/04-force-resources.sh b/tests/04-force-resources.sh index c7ca3efa..6ab9974e 100644 --- a/tests/04-force-resources.sh +++ b/tests/04-force-resources.sh @@ -9,7 +9,7 @@ longwait_failed_log="${s_basename}-failed.log" scenario_cmd() { # Encrypt file which should take a long time to decrypt. - setup_check_variables "scrypt enc 10 seconds" + setup_check "scrypt enc 10 seconds" ( echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ enc -P -t 10 "${reference_file}" \ @@ -19,7 +19,7 @@ scenario_cmd() { # Attempt to decrypt it with limited time. We want this # command to fail, so we negate the normal return code. - setup_check_variables "scrypt dec 1 second" + setup_check "scrypt dec 1 second" ( echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ dec -P -t 1 "${longwait_encrypted_file}" \ @@ -29,13 +29,13 @@ scenario_cmd() { ) # We should have received an error message. - setup_check_variables "scrypt dec 1 second error" + setup_check "scrypt dec 1 second error" grep -q "scrypt: Decrypting file would take too much CPU time" \ "${longwait_failed_log}" echo "$?" > "${c_exitfile}" # Attempt to decrypt it with limited time, but force success. - setup_check_variables "scrypt dec force" + setup_check "scrypt dec force" ( echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ dec -P -t 1 -f "${longwait_encrypted_file}" \ @@ -44,7 +44,7 @@ scenario_cmd() { ) # The decrypted reference file should match the reference. - setup_check_variables "scrypt dec force output against reference" + setup_check "scrypt dec force output against reference" cmp -s "${longwait_decrypted_file}" "${reference_file}" echo $? > "${c_exitfile}" } diff --git a/tests/05-system-scrypt-encrypt-decrypt.sh b/tests/05-system-scrypt-encrypt-decrypt.sh index 2d684666..5370322f 100644 --- a/tests/05-system-scrypt-encrypt-decrypt.sh +++ b/tests/05-system-scrypt-encrypt-decrypt.sh @@ -12,13 +12,13 @@ scenario_cmd() { if [ -z "${system_scrypt}" ]; then printf "no suitable system scrypt: " 1>&2 # Inform test suite that we are skipping. - setup_check_variables "system scrypt skip" + setup_check "system scrypt skip" echo "-1" > "${c_exitfile}" return fi # Encrypt a file with our scrypt. - setup_check_variables "scrypt enc for system" + setup_check "scrypt enc for system" ( echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ enc -P -t 1 "${reference_file}" "${encrypted_file_1}" @@ -27,7 +27,7 @@ scenario_cmd() { # Use the system scrypt to decrypt the file we just # encrypted. Don't use valgrind for this. - setup_check_variables "system scrypt dec" + setup_check "system scrypt dec" ( echo "${password}" | ${system_scrypt} \ dec -P "${encrypted_file_1}" "${decrypted_file_1}" @@ -35,13 +35,13 @@ scenario_cmd() { ) # The decrypted file should match the reference. - setup_check_variables "system scrypt dec output against reference" + setup_check "system scrypt dec output against reference" cmp -s "${decrypted_file_1}" "${reference_file}" echo $? > "${c_exitfile}" # Encrypt a file with the system scrypt. Don't use # valgrind for this. - setup_check_variables "system scrypt enc" + setup_check "system scrypt enc" ( echo "${password}" | ${system_scrypt} \ enc -P -t 1 "${reference_file}" "${encrypted_file_2}" @@ -49,7 +49,7 @@ scenario_cmd() { ) # Use our scrypt to decrypt the file we just encrypted. - setup_check_variables "scrypt dec for system" + setup_check "scrypt dec for system" ( echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ dec -P "${encrypted_file_2}" "${decrypted_file_2}" @@ -57,7 +57,7 @@ scenario_cmd() { ) # The decrypted file should match the reference. - setup_check_variables "scrypt dec for system output against reference" + setup_check "scrypt dec for system output against reference" cmp -s "${decrypted_file_2}" "${reference_file}" echo $? > "${c_exitfile}" } diff --git a/tests/06-decrypt-fail.sh b/tests/06-decrypt-fail.sh index 0a02c8e3..b3abbf75 100644 --- a/tests/06-decrypt-fail.sh +++ b/tests/06-decrypt-fail.sh @@ -9,7 +9,7 @@ non_encoded_file_output="${s_basename}-nonfile.txt" scenario_cmd() { # Attempt to decrypt a non-scrypt-encoded file. # We want this command to fail with 1. - setup_check_variables "scrypt dec non-scrypt" + setup_check "scrypt dec non-scrypt" ( echo "" | ${c_valgrind_cmd} "${bindir}/scrypt" \ dec -P "${non_encoded_file}" \ @@ -19,13 +19,13 @@ scenario_cmd() { ) # We should have received an error message. - setup_check_variables "scrypt dec non-scrypt error" + setup_check "scrypt dec non-scrypt error" grep -q "scrypt: Input is not valid scrypt-encrypted block" \ "${non_encoded_file_stderr}" echo "$?" > "${c_exitfile}" # We should not have created a file. - setup_check_variables "scrypt dec non-scrypt no file" + setup_check "scrypt dec non-scrypt no file" if [ -e "${non_encoded_file_output}" ]; then echo "1" else diff --git a/tests/07-passphrase-env.sh b/tests/07-passphrase-env.sh index 80b98d12..03d137d8 100644 --- a/tests/07-passphrase-env.sh +++ b/tests/07-passphrase-env.sh @@ -11,7 +11,7 @@ decrypted_no_envvar_log="${s_basename}-decrypt-no-envvar.log" scenario_cmd() { # Decrypt a reference file using --passphrase env:VAR. - setup_check_variables "scrypt dec env" + setup_check "scrypt dec env" PASSPHRASE="${password}" \ ${c_valgrind_cmd} "${bindir}/scrypt" \ dec --passphrase env:PASSPHRASE \ @@ -19,13 +19,13 @@ scenario_cmd() { echo $? > "${c_exitfile}" # The decrypted reference file should match the reference. - setup_check_variables "scrypt dec env output against reference" + setup_check "scrypt dec env output against reference" cmp -s "${decrypted_reference_file}" "${reference_file}" echo $? > "${c_exitfile}" # Attempt to decrypt the reference file with a non-existent envvar. # We want this command to fail with 1. - setup_check_variables "scrypt dec env none" + setup_check "scrypt dec env none" ${c_valgrind_cmd} "${bindir}/scrypt" \ dec --passphrase env:THIS_ENVVAR_DOES_NOT_EXIST \ "${encrypted_reference_file}" "${decrypted_reference_file}" \ @@ -33,20 +33,20 @@ scenario_cmd() { expected_exitcode 1 $? > "${c_exitfile}" # We should have received an error message. - setup_check_variables "scrypt dec env none error" + setup_check "scrypt dec env none error" grep -q \ "scrypt: Failed to read from \${THIS_ENVVAR_DOES_NOT_EXIST}" \ "${decrypted_no_envvar_log}" echo "$?" > "${c_exitfile}" # We should not have created a file. - setup_check_variables "scrypt dec env no file" + setup_check "scrypt dec env no file" test -e "${decrypted_badpass_file}" expected_exitcode 1 $? > "${c_exitfile}" # Attempt to decrypt the reference file with an incorrect passphrase. # We want this command to fail with 1. - setup_check_variables "scrypt dec env bad" + setup_check "scrypt dec env bad" PASSPHRASE="bad-pass" \ ${c_valgrind_cmd} "${bindir}/scrypt" \ dec --passphrase env:PASSPHRASE \ @@ -55,11 +55,11 @@ scenario_cmd() { expected_exitcode 1 $? > "${c_exitfile}" # We should have received an error message. - setup_check_variables "scrypt dec env bad error" + setup_check "scrypt dec env bad error" grep -q "scrypt: Passphrase is incorrect" "${decrypted_badpass_log}" echo "$?" > "${c_exitfile}" - setup_check_variables "scrypt dec env bad no file" + setup_check "scrypt dec env bad no file" # We should not have created a file. test -e "${decrypted_badpass_file}" expected_exitcode 1 $? > "${c_exitfile}" diff --git a/tests/08-passphrase-file.sh b/tests/08-passphrase-file.sh index 157324e6..1d1de088 100644 --- a/tests/08-passphrase-file.sh +++ b/tests/08-passphrase-file.sh @@ -16,20 +16,20 @@ scenario_cmd() { echo "${password}" > "${passphrase_file}" # Decrypt a reference file using --passphrase file:FILENAME. - setup_check_variables "scrypt dec file" + setup_check "scrypt dec file" ${c_valgrind_cmd} "${bindir}/scrypt" \ dec --passphrase file:"${passphrase_file}" \ "${encrypted_reference_file}" "${decrypted_reference_file}" echo $? > "${c_exitfile}" # The decrypted reference file should match the reference. - setup_check_variables "scrypt dec file output against reference" + setup_check "scrypt dec file output against reference" cmp -s "${decrypted_reference_file}" "${reference_file}" echo $? > "${c_exitfile}" # Attempt to decrypt the reference file with a non-existent file. # We want this command to fail with 1. - setup_check_variables "scrypt dec file none" + setup_check "scrypt dec file none" ${c_valgrind_cmd} "${bindir}/scrypt" \ dec --passphrase file:THIS_FILE_DOES_NOT_EXIST \ "${encrypted_reference_file}" "${decrypted_reference_file}" \ @@ -37,19 +37,19 @@ scenario_cmd() { expected_exitcode 1 $? > "${c_exitfile}" # We should have received an error message. - setup_check_variables "scrypt dec file none error" + setup_check "scrypt dec file none error" grep -q "scrypt: fopen(THIS_FILE_DOES_NOT_EXIST)" \ "${decrypted_no_file_log}" echo "$?" > "${c_exitfile}" # We should not have created a file. - setup_check_variables "scrypt dec file none no file" + setup_check "scrypt dec file none no file" test -e "${decrypted_badpass_file}" expected_exitcode 1 $? > "${c_exitfile}" # Attempt to decrypt the reference file with an incorrect passphrase. # We want this command to fail with 1. - setup_check_variables "scrypt dec file bad" + setup_check "scrypt dec file bad" echo "bad-pass" > "${bad_passphrase_file}" ${c_valgrind_cmd} "${bindir}/scrypt" \ dec --passphrase file:"${bad_passphrase_file}" \ @@ -58,12 +58,12 @@ scenario_cmd() { expected_exitcode 1 $? > "${c_exitfile}" # We should have received an error message. - setup_check_variables "scrypt dec file bad error" + setup_check "scrypt dec file bad error" grep -q "scrypt: Passphrase is incorrect" "${decrypted_badpass_log}" echo "$?" > "${c_exitfile}" # We should not have created a file. - setup_check_variables "scrypt dec file bad no file" + setup_check "scrypt dec file bad no file" test -e "${decrypted_badpass_file}" expected_exitcode 1 $? > "${c_exitfile}" } diff --git a/tests/09-explicit-params.sh b/tests/09-explicit-params.sh index 9a0e410b..7b75b05b 100644 --- a/tests/09-explicit-params.sh +++ b/tests/09-explicit-params.sh @@ -10,7 +10,7 @@ stderr_bad="${s_basename}-reference-bad.stderr" scenario_cmd() { # Encrypt with manually-specified N, r, p. - setup_check_variables "scrypt enc Nrp" + setup_check "scrypt enc Nrp" echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ enc -v --logN 12 -r 2 -p 3 \ --passphrase dev:stdin-once \ @@ -19,20 +19,20 @@ scenario_cmd() { echo $? > "${c_exitfile}" # Check that the options were used. - setup_check_variables "scrypt enc Nrp output N" + setup_check "scrypt enc Nrp output N" grep -q "N = 4096" "${stderr}" echo $? > "${c_exitfile}" - setup_check_variables "scrypt enc Nrp output r" + setup_check "scrypt enc Nrp output r" grep -q "r = 2" "${stderr}" echo $? > "${c_exitfile}" - setup_check_variables "scrypt enc Nrp output p" + setup_check "scrypt enc Nrp output p" grep -q "p = 3" "${stderr}" echo $? > "${c_exitfile}" # Try to encrypt with badly-specified N, r, p; should fail. - setup_check_variables "scrypt enc Nrp bad" + setup_check "scrypt enc Nrp bad" echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ enc -v --logN 2 -r 0 -p 0 \ --passphrase dev:stdin-once \ @@ -41,24 +41,24 @@ scenario_cmd() { expected_exitcode 1 $? > "${c_exitfile}" # Check that we got an error. - setup_check_variables "scrypt enc Nrp bad output" + setup_check "scrypt enc Nrp bad output" grep -q "\--logN must be between 10 and 40 (inclusive)" "${stderr_bad}" echo $? > "${c_exitfile}" # Check that we can't partially set explicit parameters. - setup_check_variables "scrypt enc --logN only" + setup_check "scrypt enc --logN only" ${c_valgrind_cmd} "${bindir}/scrypt" \ enc --logN 12 "${reference_file}" 2>&1 | \ grep -q "If --logN is set, -r and -p must also be set" echo $? > "${c_exitfile}" - setup_check_variables "scrypt enc -r only" + setup_check "scrypt enc -r only" ${c_valgrind_cmd} "${bindir}/scrypt" \ enc -r 12 "${reference_file}" 2>&1 | \ grep -q "If -r is set, --logN and -p must also be set" echo $? > "${c_exitfile}" - setup_check_variables "scrypt enc -p only" + setup_check "scrypt enc -p only" ${c_valgrind_cmd} "${bindir}/scrypt" \ enc -p 12 "${reference_file}" 2>&1 | \ grep -q "If -p is set, --logN and -r must also be set" diff --git a/tests/shared_test_functions.sh b/tests/shared_test_functions.sh index ffedc094..1f48a466 100644 --- a/tests/shared_test_functions.sh +++ b/tests/shared_test_functions.sh @@ -24,7 +24,7 @@ # Look for a ${cmd} in $(ps). # - wait_while(func): # Wait until ${func} returns non-zero. -# - setup_check_variables(description, check_prev): +# - setup_check(description, check_prev): # Set up the below variables. # - expected_exitcode(expected, actual): # Check if ${expected} matches ${actual}. @@ -155,14 +155,14 @@ wait_while() { return 0 } -## setup_check_variables (description, check_prev=1): +## setup_check (description, check_prev=1): # Set up the "check" variables ${c_exitfile} and ${c_valgrind_cmd}, the # latter depending on the previously-defined ${c_valgrind_min}. # Advance the number of checks ${c_count_next} so that the next call to this # function will set up new filenames. Write ${description} into a # file. If ${check_prev} is non-zero, check that the previous # ${c_exitfile} exists. -setup_check_variables() { +setup_check() { description=$1 check_prev=${2:-1} @@ -188,7 +188,7 @@ setup_check_variables() { "${s_basename}-${c_count_str}.desc" # Set up the valgrind command (or an empty string). - c_valgrind_cmd="$(valgrind_setup_cmd)" + c_valgrind_cmd="$(valgrind_setup)" # Advances the number of checks. c_count_next=$((c_count_next + 1)) @@ -261,7 +261,7 @@ _notify_success_or_fail() { fi # Check valgrind logfile(s). - val_failed="$(valgrind_check_basenames "${exitfile}")" + val_failed="$(valgrind_check "${exitfile}")" if [ -n "${val_failed}" ]; then echo "FAILED!" 1>&2 s_retval="${valgrind_exit_code}" diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh index 83ad7f35..d5588c42 100644 --- a/tests/shared_valgrind_functions.sh +++ b/tests/shared_valgrind_functions.sh @@ -10,10 +10,10 @@ set -o noclobber -o nounset # - valgrind_init(): # Clear previous valgrind output, and prepare for running valgrind tests # (if applicable). -# - valgrind_setup_cmd(str): +# - valgrind_setup(str): # Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to # ${valgrind_min}. If ${str} is not blank, include it in the log filename. -# - valgrind_check_basenames(exitfile): +# - valgrind_check(exitfile): # Check for any memory leaks recorded in valgrind logfiles associated with a # test exitfile. Return the filename if there's a leak; otherwise return an # empty string. @@ -223,10 +223,10 @@ _valgrind_ensure_suppression() { printf "done.\n" 1>&2 } -## valgrind_setup_cmd (str): +## valgrind_setup (str): # Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to # ${valgrind_min}. If ${str} is not blank, include it in the log filename. -valgrind_setup_cmd() { +valgrind_setup() { str=${1:-} # Bail if we don't want to use valgrind for this check. @@ -332,11 +332,11 @@ _valgrind_check_logfile() { fi } -## valgrind_check_basenames (exitfile): +## valgrind_check (exitfile): # Check for any memory leaks recorded in valgrind logfiles associated with a # test exitfile. Return the filename if there's a leak; otherwise return an # empty string. -valgrind_check_basenames() { +valgrind_check() { exitfile="$1" val_basename=$(_valgrind_get_basename "${exitfile}") From 18829b32c8c5508e01ecbd21ba96d531d7bcf4ad Mon Sep 17 00:00:00 2001 From: Graham Percival Date: Wed, 11 Oct 2023 19:44:14 -0700 Subject: [PATCH 2/4] tests: shorten some function names in the private API Changes to shared_test_functions.sh: s/_prepare_directory/_prepdir/g s/_notify_success_or_fail/_check/g Changes to shared_valgrind_functions.sh: s/_valgrind_prep_directory/_val_prepdir/g s/_valgrind_check_optional/_val_checkver/g s/_valgrind_process_suppression_file/_val_generalize/g s/_valgrind_ensure_suppression/_val_ensure/g s/_valgrind_get_basename/_val_getbase/g s/_valgrind_check_logfile/_val_checkl/g --- tests/shared_test_functions.sh | 12 ++++----- tests/shared_valgrind_functions.sh | 41 +++++++++++++++--------------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/tests/shared_test_functions.sh b/tests/shared_test_functions.sh index 1f48a466..860c0017 100644 --- a/tests/shared_test_functions.sh +++ b/tests/shared_test_functions.sh @@ -77,9 +77,9 @@ bindir=$(CDPATH='' cd -- "$(dirname -- "${1-.}")" && pwd -P) NO_EXITFILE=/dev/null -## _prepare_directory(): +## _prepdir(): # Delete the previous test output directory, and create a new one. -_prepare_directory() { +_prepdir() { if [ -d "${out}" ]; then rm -rf "${out}" fi @@ -211,14 +211,14 @@ expected_exitcode() { fi } -## _notify_success_or_fail (log_basename, val_log_basename): +## _check (log_basename, val_log_basename): # Examine all "exit code" files beginning with ${log_basename} and # print "SUCCESS!", "FAILED!", "SKIP!", or "PARTIAL SUCCESS / SKIP!" # as appropriate. Check any valgrind log files associated with the # test and print "FAILED!" if appropriate, along with the valgrind # logfile. If the test failed and ${VERBOSE} is non-zero, print # the description to stderr. -_notify_success_or_fail() { +_check() { log_basename=$1 val_log_basename=$2 @@ -311,7 +311,7 @@ _scenario_runner() { # Print PASS or FAIL, and return result. s_retval=0 - _notify_success_or_fail "${s_basename}" "${s_val_basename}" + _check "${s_basename}" "${s_val_basename}" return "${s_retval}" } @@ -328,7 +328,7 @@ run_scenarios() { fi # Clean up any previous directory, and create a new one. - _prepare_directory + _prepdir # Clean up any previous valgrind directory, and prepare for new # valgrind tests (if applicable). diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh index d5588c42..4be339b8 100644 --- a/tests/shared_valgrind_functions.sh +++ b/tests/shared_valgrind_functions.sh @@ -33,10 +33,10 @@ set -o noclobber -o nounset # tested. valgrind_exit_code=108 -## _valgrind_prepare_directory (): +## _val_prepdir (): # Clean up a previous valgrind directory, and prepare for new valgrind tests # (if applicable). -_valgrind_prepare_directory() { +_val_prepdir() { # If we don't want to generate new suppressions files, move them. if [ "${USE_VALGRIND_NO_REGEN}" -gt 0 ]; then # Bail if the file doesn't exist. @@ -75,10 +75,10 @@ _valgrind_prepare_directory() { [ "$(uname)" = "FreeBSD" ] && chflags nodump "${out_valgrind}" } -## _valgrind_check_optional (): +## _val_checkver (): # If ${USE_VALGRIND} is greater than 0, check that valgrind is available in # the ${PATH} and is at least version 3.13. -_valgrind_check_optional() { +_val_checkver() { if [ "${USE_VALGRIND}" -gt 0 ]; then # Look for valgrind in ${PATH}. if ! command -v valgrind >/dev/null 2>&1; then @@ -101,10 +101,10 @@ _valgrind_check_optional() { fi } -## _valgrind_process_suppression_file(filename): +## _val_generalize(filename): # Generalize suppressions from a valgrind suppression file by omitting the # "fun:pl_*" and "fun:main" lines and anything below them. -_valgrind_process_suppression_file() { +_val_generalize() { filename=$1 # How many segments do we have? @@ -165,12 +165,12 @@ _valgrind_process_suppression_file() { done } -## _valgrind_ensure_suppression (potential_memleaks_binary): +## _val_ensure (potential_memleaks_binary): # Run the ${potential_memleaks_binary} through valgrind, keeping # track of any apparent memory leak in order to suppress reporting # those leaks when testing other binaries. Record a log file which shows the # open file descriptors in ${valgrind_fds_log}. -_valgrind_ensure_suppression() { +_val_ensure() { potential_memleaks_binary=$1 # Quit if we're not using valgrind. @@ -215,7 +215,7 @@ _valgrind_ensure_suppression() { # Strip out useless parts from the log file, and allow the # suppressions to apply to other binaries. - _valgrind_process_suppression_file "${this_valgrind_supp}" + _val_generalize "${this_valgrind_supp}" done < "${valgrind_suppressions_tests}" # Clean up @@ -264,19 +264,19 @@ valgrind_incomplete() { test -n "${_valgrind_incomplete_logfiles}" } -## _valgrind_get_basename (exitfile): +## _val_getbase (exitfile): # Return the filename without ".log" of the valgrind logfile corresponding to # ${exitfile}. -_valgrind_get_basename() { +_val_getbase() { exitfile=$1 basename=$(basename "${exitfile}" ".exit") echo "${out_valgrind}/${basename}" } -## _valgrind_check_logfile(logfile) +## _val_checkl(logfile) # Check for any (unsuppressed) memory leaks recorded in a valgrind logfile. # Echo the filename if there's a leak; otherwise, echo nothing. -_valgrind_check_logfile() { +_val_checkl() { logfile=$1 # Bytes in use at exit. @@ -338,7 +338,7 @@ _valgrind_check_logfile() { # empty string. valgrind_check() { exitfile="$1" - val_basename=$(_valgrind_get_basename "${exitfile}") + val_basename=$(_val_getbase "${exitfile}") # Get list of files to check. (Yes, the star goes outside the quotes.) logfiles=$(ls "${val_basename}"* 2>/dev/null) @@ -352,7 +352,7 @@ valgrind_check() { # Check a single file. if [ "${num_logfiles}" -eq "1" ]; then - _valgrind_check_logfile "${logfiles}" + _val_checkl "${logfiles}" return fi @@ -361,7 +361,7 @@ valgrind_check() { for logfile in ${logfiles} ; do if [ "${logfile#*-valgrind-parent-}" != "${logfile}" ]; then # Only check the parent - _valgrind_check_logfile "${logfile}" + _val_checkl "${logfile}" return "$?" fi done @@ -382,7 +382,7 @@ valgrind_check() { awk '{ print $4 }') if [ "${val_pids#*"${val_parent_pid}"}" != \ "${val_pids}" ]; then - _valgrind_check_logfile "${logfile}" + _val_checkl "${logfile}" return "$?" fi done @@ -402,13 +402,12 @@ valgrind_init() { valgrind_fds_log="${out_valgrind}/fds.log" # If we want valgrind, check that the version is high enough. - _valgrind_check_optional + _val_checkver # Remove any previous directory, and create a new one. - _valgrind_prepare_directory + _val_prepdir # Generate valgrind suppression file if it is required. Must be # done after preparing the directory. - _valgrind_ensure_suppression \ - "${bindir}/tests/valgrind/potential-memleaks" + _val_ensure "${bindir}/tests/valgrind/potential-memleaks" } From 433bd4244e0058194eb0824f528cfeae49acbe3c Mon Sep 17 00:00:00 2001 From: Graham Percival Date: Wed, 11 Oct 2023 19:44:37 -0700 Subject: [PATCH 3/4] tests: minor tweaks and readability reformatting - ${valgrind_exit_code}: move to shared_test_functions.sh, since it's only used in that file. - _val_checkver(): bail early if we're not using valgrind; that lets us eliminate one tab level in the rest of the function. - _val_seg(): add new function, moving code out of _val_generalize(). --- tests/shared_test_functions.sh | 8 ++- tests/shared_valgrind_functions.sh | 111 +++++++++++++++-------------- 2 files changed, 65 insertions(+), 54 deletions(-) diff --git a/tests/shared_test_functions.sh b/tests/shared_test_functions.sh index 860c0017..89191c5f 100644 --- a/tests/shared_test_functions.sh +++ b/tests/shared_test_functions.sh @@ -76,6 +76,10 @@ bindir=$(CDPATH='' cd -- "$(dirname -- "${1-.}")" && pwd -P) # Default value (should be set by tests). NO_EXITFILE=/dev/null +# A non-zero value unlikely to be used as an exit code by the programs being +# tested. +valgrind_exit_code=108 + ## _prepdir(): # Delete the previous test output directory, and create a new one. @@ -247,8 +251,8 @@ _check() { if [ "${ret}" -gt 0 ]; then echo "FAILED!" 1>&2 if [ "${VERBOSE}" -ne 0 ]; then - printf "File %s contains" "${exitfile}" 1>&2 - printf " exit code %s.\n" "${ret}" 1>&2 + printf "File %s contains exit code %s.\n" \ + "${exitfile}" "${ret}" 1>&2 printf "Test description: " 1>&2 cat "${descfile}" 1>&2 fi diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh index 4be339b8..b30f5cd5 100644 --- a/tests/shared_valgrind_functions.sh +++ b/tests/shared_valgrind_functions.sh @@ -29,9 +29,6 @@ set -o noclobber -o nounset # - valgrind_suppressions: filename of valgrind suppressions. # - valgrind_fds_log: filename of the log of open file descriptors. -# A non-zero value unlikely to be used as an exit code by the programs being -# tested. -valgrind_exit_code=108 ## _val_prepdir (): # Clean up a previous valgrind directory, and prepare for new valgrind tests @@ -79,26 +76,65 @@ _val_prepdir() { # If ${USE_VALGRIND} is greater than 0, check that valgrind is available in # the ${PATH} and is at least version 3.13. _val_checkver() { - 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 - exit 1 - fi + # Quit if we're not using valgrind. + if [ ! "${USE_VALGRIND}" -gt 0 ]; then + return + fi; + + # Look for valgrind in $PATH. + if ! command -v valgrind >/dev/null 2>&1; then + printf "valgrind not found\n" 1>&2 + exit 1 + fi + + # Check the version. + version=$(valgrind --version | cut -d "-" -f 2) + major=$(echo "${version}" | cut -d "." -f 1) + minor=$(echo "${version}" | cut -d "." -f 2) + if [ "${major}" -lt "3" ]; then + printf "valgrind must be at least version 3.13\n" 1>&2 + exit 1; + fi + if [ "${major}" -eq "3" ] && [ "${minor}" -lt "13" ]; then + printf "valgrind must be at least version 3.13\n" 1>&2 + exit 1; + fi +} - # Check the version. - version=$(valgrind --version | cut -d "-" -f 2) - major=$(echo "${version}" | cut -d "." -f 1) - minor=$(echo "${version}" | cut -d "." -f 2) - if [ "${major}" -lt "3" ]; then - printf "valgrind must be at least version 3.13\n" 1>&2 - exit 1; +## _val_seg(filename): +# Generalize an already-segmented portion of a valgrind suppressions file; +# write the result to ${valgrind_suppressions}. +_val_seg() { + _val_seg_filename=$1 + + # Find last relevant line. + lastline="$(grep -n "}" "${_val_seg_filename}" | cut -f1 -d:)" + + # Cut off anything below the 1st "fun:pl_" (inclusive). + funcline="$(grep -n "fun:pl_" "${_val_seg_filename}" | \ + cut -f1 -d: | \ + head -n1)" + if [ -n "${funcline}" ]; then + if [ "${lastline}" -gt "${funcline}" ]; then + lastline="${funcline}" fi - if [ "${major}" -eq "3" ] && [ "${minor}" -lt "13" ]; then - printf "valgrind must be at least version 3.13\n" 1>&2 - exit 1; + fi + + # Cut off anything below "fun:main" (including that line). (Due to + # linking and/or optimizations, some memory leaks occur without + # "fun:pl_" appearing in the valgrind suppression.) + funcline="$(grep -n "fun:main" "${_val_seg_filename}" | cut -f1 -d:)" + if [ -n "${funcline}" ]; then + if [ "${lastline}" -gt "${funcline}" ]; then + lastline="${funcline}" fi fi + + # Only keep the beginning of each suppression. + lastline="$((lastline - 1))" + head -n "${lastline}" "${_val_seg_filename}" >> \ + "${valgrind_suppressions}" + printf "}\n" >> "${valgrind_suppressions}" } ## _val_generalize(filename): @@ -122,43 +158,14 @@ _val_generalize() { fi # Split into segments. - csplit -f "${filename}" "${filename}" "/{/" \ - "{$((num_segments - 1))}" > /dev/null + csplit -f "${filename}" "${filename}" \ + "/{/" "{$((num_segments - 1))}" > /dev/null # Skip "${filename}00" because that doesn't contain a suppression. i=1 while [ "${i}" -le "${num_segments}" ]; do - segfilename="$(printf "%s%02i" "${filename}" "${i}")" - - # Find last relevant line. - lastline="$(grep -n "}" "${segfilename}" | cut -f1 -d:)" - - # Cut off anything below the 1st "fun:pl_" (inclusive). - funcline="$(grep -n "fun:pl_" "${segfilename}" | \ - cut -f1 -d: | \ - head -n1)" - if [ -n "${funcline}" ]; then - if [ "${lastline}" -gt "${funcline}" ]; then - lastline="${funcline}" - fi - fi - - # Cut off anything below "fun:main" (including that line). - # (Due to linking and/or optimizations, some memory leaks - # occur without "fun:pl_" appearing in the valgrind - # suppression.) - funcline="$(grep -n "fun:main" "${segfilename}" | cut -f1 -d:)" - if [ -n "${funcline}" ]; then - if [ "${lastline}" -gt "${funcline}" ]; then - lastline="${funcline}" - fi - fi - - # Only keep the beginning of each suppression. - lastline="$((lastline - 1))" - head -n "${lastline}" "${segfilename}" >> \ - "${valgrind_suppressions}" - printf "}\n" >> "${valgrind_suppressions}" + # Process segment + _val_seg "$(printf "%s%02i" "${filename}" "${i}")" # Advance to the next suppression. i=$((i + 1)) From 171627ab6fe880c3e24733f08c519835cb860ac1 Mon Sep 17 00:00:00 2001 From: Graham Percival Date: Wed, 11 Oct 2023 19:46:43 -0700 Subject: [PATCH 4/4] tests: use "local" variables POSIX sh does not include local variables [1], so we're switching to the convention of using: _func_var_name for local function variable names, so that we always have distinct names. This does lead to rather verbose variable names, but this is an acceptable trade-off in order to have correct program behaviour. [1] Adding "local" to POSIX was discussed, but rejected, in: https://www.austingroupbugs.net/view.php?id=767 It's possible to get "local variables" by abusing eval, but those solutions don't look appealing. --- tests/shared_test_functions.sh | 123 ++++++++++--------- tests/shared_valgrind_functions.sh | 183 +++++++++++++++-------------- 2 files changed, 159 insertions(+), 147 deletions(-) diff --git a/tests/shared_test_functions.sh b/tests/shared_test_functions.sh index 89191c5f..d4a12924 100644 --- a/tests/shared_test_functions.sh +++ b/tests/shared_test_functions.sh @@ -96,8 +96,8 @@ _prepdir() { ## find_system (cmd, args): # Look for ${cmd} in the ${PATH}, and ensure that it supports ${args}. find_system() { - cmd=$1 - cmd_with_args="$1 ${2:-}" + _find_system_cmd=$1 + _find_system_cmd_with_args="$1 ${2:-}" # Sanity check. if [ "$#" -gt "2" ]; then @@ -106,26 +106,28 @@ find_system() { fi # Look for ${cmd}; the "|| true" and -} make this work with set -e. - system_binary=$(command -v "${cmd}") || true - if [ -z "${system_binary-}" ]; then - system_binary="" - printf "System %s not found.\n" "${cmd}" 1>&2 + _find_system_binary=$(command -v "${_find_system_cmd}") || true + if [ -z "${_find_system_binary-}" ]; then + _find_system_binary="" + printf "System %s not found.\n" "${_find_system_cmd}" 1>&2 # If the command exists, check it ensures the ${args}. - elif ${cmd_with_args} 2>&1 >/dev/null | \ + elif ${_find_system_cmd_with_args} 2>&1 >/dev/null | \ grep -qE "(invalid|illegal) option"; then - system_binary="" - printf "Cannot use system %s; does not" "${cmd}" 1>&2 + _find_system_binary="" + printf "Cannot use system %s; does not" \ + "${_find_system_cmd}" 1>&2 printf " support necessary arguments.\n" 1>&2 fi - echo "${system_binary}" + echo "${_find_system_binary}" } ## has_pid (cmd): # Look for ${cmd} in ps; return 0 if ${cmd} exists. has_pid() { - cmd=$1 - pid=$(ps -Aopid,args | grep -F "${cmd}" | grep -v "grep") || true - if [ -n "${pid}" ]; then + _has_pid_cmd=$1 + _has_pid_pid=$(ps -Aopid,args | grep -F "${_has_pid_cmd}" | \ + grep -v "grep") || true + if [ -n "${_has_pid_pid}" ]; then return 0 fi return 1 @@ -167,12 +169,12 @@ wait_while() { # file. If ${check_prev} is non-zero, check that the previous # ${c_exitfile} exists. setup_check() { - description=$1 - check_prev=${2:-1} + _setup_check_description=$1 + _setup_check_prev=${2:-1} # Should we check for the previous exitfile? if [ "${c_exitfile}" != "${NO_EXITFILE}" ] && \ - [ "${check_prev}" -gt 0 ] ; then + [ "${_setup_check_prev}" -gt 0 ] ; then # Check for the file. if [ ! -f "${c_exitfile}" ] ; then # We should have written the result of the @@ -188,7 +190,7 @@ setup_check() { c_exitfile="${s_basename}-${c_count_str}.exit" # Write the "description" file. - printf "%s\n" "${description}" > \ + printf "%s\n" "${_setup_check_description}" > \ "${s_basename}-${c_count_str}.desc" # Set up the valgrind command (or an empty string). @@ -203,12 +205,14 @@ setup_check() { # ${valgrind_exit_code}, return that. Otherwise, return 1 to indicate # failure. expected_exitcode() { - expected=$1 - exitcode=$2 + _expected_exitcode_expected=$1 + _expected_exitcode_exitcode=$2 - if [ "${exitcode}" -eq "${expected}" ]; then + if [ "${_expected_exitcode_exitcode}" -eq \ + "${_expected_exitcode_expected}" ]; then echo "0" - elif [ "${exitcode}" -eq "${valgrind_exit_code}" ]; then + elif [ "${_expected_exitcode_exitcode}" -eq \ + "${valgrind_exit_code}" ]; then echo "${valgrind_exit_code}" else echo "1" @@ -223,60 +227,62 @@ expected_exitcode() { # logfile. If the test failed and ${VERBOSE} is non-zero, print # the description to stderr. _check() { - log_basename=$1 - val_log_basename=$2 + _check_log_basename=$1 + _check_val_log_basename=$2 # Bail if there's no exitfiles. - exitfiles=$(ls "${log_basename}"-*.exit) || true - if [ -z "${exitfiles}" ]; then + _check_exitfiles=$(ls "${_check_log_basename}"-*.exit) || true + if [ -z "${_check_exitfiles}" ]; then echo "FAILED" 1>&2 s_retval=1 return fi # Count results - total_exitfiles=0 - skip_exitfiles=0 + _check_total=0 + _check_skip=0 # Check each exitfile. - for exitfile in $(echo "${exitfiles}" | sort); do - ret=$(cat "${exitfile}") - total_exitfiles=$(( total_exitfiles + 1 )) - if [ "${ret}" -lt 0 ]; then - skip_exitfiles=$(( skip_exitfiles + 1 )) + for _check_exitfile in $(echo "${_check_exitfiles}" | sort); do + _check_ret=$(cat "${_check_exitfile}") + _check_total=$(( _check_total + 1 )) + if [ "${_check_ret}" -lt 0 ]; then + _check_skip=$(( _check_skip + 1 )) fi # Check for test failure. - descfile=$(echo "${exitfile}" | sed 's/\.exit/\.desc/g') - if [ "${ret}" -gt 0 ]; then + _check_descfile=$(echo "${_check_exitfile}" \ + | sed 's/\.exit/\.desc/g') + if [ "${_check_ret}" -gt 0 ]; then echo "FAILED!" 1>&2 if [ "${VERBOSE}" -ne 0 ]; then printf "File %s contains exit code %s.\n" \ - "${exitfile}" "${ret}" 1>&2 + "${_check_exitfile}" "${_check_ret}" \ + 1>&2 printf "Test description: " 1>&2 - cat "${descfile}" 1>&2 + cat "${_check_descfile}" 1>&2 fi - s_retval=${ret} + s_retval=${_check_ret} return else # If there's no failure, delete the files. - rm "${exitfile}" - rm "${descfile}" + rm "${_check_exitfile}" + rm "${_check_descfile}" fi # Check valgrind logfile(s). - val_failed="$(valgrind_check "${exitfile}")" - if [ -n "${val_failed}" ]; then + _check_val_failed="$(valgrind_check "${_check_exitfile}")" + if [ -n "${_check_val_failed}" ]; then echo "FAILED!" 1>&2 s_retval="${valgrind_exit_code}" - cat "${val_failed}" 1>&2 + cat "${_check_val_failed}" 1>&2 return fi done # Notify about skip or success. - if [ "${skip_exitfiles}" -gt 0 ]; then - if [ "${skip_exitfiles}" -eq "${total_exitfiles}" ]; then + if [ "${_check_skip}" -gt 0 ]; then + if [ "${_check_skip}" -eq "${_check_total}" ]; then echo "SKIP!" 1>&2 else echo "PARTIAL SUCCESS / SKIP!" 1>&2 @@ -289,13 +295,13 @@ _check() { ## _scenario_runner (scenario_filename): # Run a test scenario from ${scenario_filename}. _scenario_runner() { - scenario_filename=$1 - basename=$(basename "${scenario_filename}" .sh) - printf " %s... " "${basename}" 1>&2 + _scenario_runner_filename=$1 + _scenario_runner_basename=$(basename "${_scenario_runner_filename}" .sh) + printf " %s... " "${_scenario_runner_basename}" 1>&2 # Initialize "scenario" and "check" variables. - s_basename=${out}/${basename} - s_val_basename=${out_valgrind}/${basename} + s_basename=${out}/${_scenario_runner_basename} + s_val_basename=${out_valgrind}/${_scenario_runner_basename} c_count_next=0 c_exitfile="${NO_EXITFILE}" c_valgrind_min=9 @@ -303,10 +309,10 @@ _scenario_runner() { # Load scenario_cmd() from the scenario file. unset scenario_cmd - . "${scenario_filename}" + . "${_scenario_runner_filename}" if ! command -v scenario_cmd 1>/dev/null ; then printf "ERROR: scenario_cmd() is not defined in\n" 1>&2 - printf " %s\n" "${scenario_filename}" 1>&2 + printf " %s\n" "${_scenario_runner_filename}" 1>&2 exit 1 fi @@ -326,9 +332,10 @@ _scenario_runner() { run_scenarios() { # Get the test number(s) to run. if [ "${N:-0}" -gt "0" ]; then - test_scenarios="$(printf "${scriptdir}/%02d-*.sh" "${N}")" + _run_scenarios_filenames="$(printf \ + "${scriptdir}/%02d-*.sh" "${N}")" else - test_scenarios="${scriptdir}/??-*.sh" + _run_scenarios_filenames="${scriptdir}/??-*.sh" fi # Clean up any previous directory, and create a new one. @@ -340,13 +347,13 @@ run_scenarios() { printf -- "Running tests\n" 1>&2 printf -- "-------------\n" 1>&2 - for scenario in ${test_scenarios}; do + for _run_scenarios_filename in ${_run_scenarios_filenames}; do # We can't call this function with $( ... ) because we # want to allow it to echo values to stdout. - _scenario_runner "${scenario}" - retval=$? - if [ "${retval}" -gt 0 ]; then - exit "${retval}" + _scenario_runner "${_run_scenarios_filename}" + _run_scenarios_retval=$? + if [ "${_run_scenarios_retval}" -gt 0 ]; then + exit "${_run_scenarios_retval}" fi done } diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh index b30f5cd5..2cd971b2 100644 --- a/tests/shared_valgrind_functions.sh +++ b/tests/shared_valgrind_functions.sh @@ -43,10 +43,10 @@ _val_prepdir() { fi # Move the files away. - supp_tmp="$(mktemp /tmp/valgrind-suppressions.XXXXXX)" - fds_tmp="$(mktemp /tmp/valgrind-fds.XXXXXX)" - mv "${valgrind_suppressions}" "${supp_tmp}" - mv "${valgrind_fds_log}" "${fds_tmp}" + _val_prepdir_supp_tmp="$(mktemp /tmp/valgrind-suppressions.XXXXXX)" + _val_prepdir_fds_tmp="$(mktemp /tmp/valgrind-fds.XXXXXX)" + mv "${valgrind_suppressions}" "${_val_prepdir_supp_tmp}" + mv "${valgrind_fds_log}" "${_val_prepdir_fds_tmp}" fi # Always delete any previous valgrind directory. @@ -64,8 +64,8 @@ _val_prepdir() { # If we don't want to generate a new suppressions file, restore it. if [ "${USE_VALGRIND_NO_REGEN}" -gt 0 ]; then # Move the files back. - mv "${supp_tmp}" "${valgrind_suppressions}" - mv "${fds_tmp}" "${valgrind_fds_log}" + mv "${_val_prepdir_supp_tmp}" "${valgrind_suppressions}" + mv "${_val_prepdir_fds_tmp}" "${valgrind_fds_log}" fi # We don't want to back up this directory. @@ -88,14 +88,15 @@ _val_checkver() { fi # Check the version. - version=$(valgrind --version | cut -d "-" -f 2) - major=$(echo "${version}" | cut -d "." -f 1) - minor=$(echo "${version}" | cut -d "." -f 2) - if [ "${major}" -lt "3" ]; then + _val_checkver_version=$(valgrind --version | cut -d "-" -f 2) + _val_checkver_major=$(echo "${_val_checkver_version}" | cut -d "." -f 1) + _val_checkver_minor=$(echo "${_val_checkver_version}" | cut -d "." -f 2) + if [ "${_val_checkver_major}" -lt "3" ]; then printf "valgrind must be at least version 3.13\n" 1>&2 exit 1; fi - if [ "${major}" -eq "3" ] && [ "${minor}" -lt "13" ]; then + if [ "${_val_checkver_major}" -eq "3" ] && \ + [ "${_val_checkver_minor}" -lt "13" ]; then printf "valgrind must be at least version 3.13\n" 1>&2 exit 1; fi @@ -108,31 +109,32 @@ _val_seg() { _val_seg_filename=$1 # Find last relevant line. - lastline="$(grep -n "}" "${_val_seg_filename}" | cut -f1 -d:)" + _val_seg_lastline="$(grep -n "}" "${_val_seg_filename}" | cut -f1 -d:)" # Cut off anything below the 1st "fun:pl_" (inclusive). - funcline="$(grep -n "fun:pl_" "${_val_seg_filename}" | \ + _val_seg_funcline="$(grep -n "fun:pl_" "${_val_seg_filename}" | \ cut -f1 -d: | \ head -n1)" - if [ -n "${funcline}" ]; then - if [ "${lastline}" -gt "${funcline}" ]; then - lastline="${funcline}" + if [ -n "${_val_seg_funcline}" ]; then + if [ "${_val_seg_lastline}" -gt "${_val_seg_funcline}" ]; then + _val_seg_lastline="${_val_seg_funcline}" fi fi # Cut off anything below "fun:main" (including that line). (Due to # linking and/or optimizations, some memory leaks occur without # "fun:pl_" appearing in the valgrind suppression.) - funcline="$(grep -n "fun:main" "${_val_seg_filename}" | cut -f1 -d:)" - if [ -n "${funcline}" ]; then - if [ "${lastline}" -gt "${funcline}" ]; then - lastline="${funcline}" + _val_seg_funcline="$(grep -n "fun:main" "${_val_seg_filename}" | \ + cut -f1 -d:)" + if [ -n "${_val_seg_funcline}" ]; then + if [ "${_val_seg_lastline}" -gt "${_val_seg_funcline}" ]; then + _val_seg_lastline="${_val_seg_funcline}" fi fi # Only keep the beginning of each suppression. - lastline="$((lastline - 1))" - head -n "${lastline}" "${_val_seg_filename}" >> \ + _val_seg_lastline="$((_val_seg_lastline - 1))" + head -n "${_val_seg_lastline}" "${_val_seg_filename}" >> \ "${valgrind_suppressions}" printf "}\n" >> "${valgrind_suppressions}" } @@ -141,34 +143,35 @@ _val_seg() { # Generalize suppressions from a valgrind suppression file by omitting the # "fun:pl_*" and "fun:main" lines and anything below them. _val_generalize() { - filename=$1 + _val_generalize_filename=$1 # How many segments do we have? - num_segments="$(grep -c "^{" "${filename}")" + _val_generalize_num_segments="$(grep -c "^{" "${_val_generalize_filename}")" # Bail if there's nothing to do. - if [ "${num_segments}" -eq "0" ]; then + if [ "${_val_generalize_num_segments}" -eq "0" ]; then return fi # Sanity check. - if [ "${num_segments}" -gt 100 ]; then + if [ "${_val_generalize_num_segments}" -gt 100 ]; then printf "More than 100 valgrind suppressions?!\n" 1>&2 exit 1 fi # Split into segments. - csplit -f "${filename}" "${filename}" \ - "/{/" "{$((num_segments - 1))}" > /dev/null + csplit -f "${_val_generalize_filename}" "${_val_generalize_filename}" \ + "/{/" "{$((_val_generalize_num_segments - 1))}" > /dev/null # Skip "${filename}00" because that doesn't contain a suppression. - i=1 - while [ "${i}" -le "${num_segments}" ]; do + _val_generalize_i=1 + while [ "${_val_generalize_i}" -le "${_val_generalize_num_segments}" ]; do # Process segment - _val_seg "$(printf "%s%02i" "${filename}" "${i}")" + _val_seg "$(printf "%s%02i" \ + "${_val_generalize_filename}" "${_val_generalize_i}")" # Advance to the next suppression. - i=$((i + 1)) + _val_generalize_i=$((_val_generalize_i + 1)) done } @@ -178,7 +181,7 @@ _val_generalize() { # those leaks when testing other binaries. Record a log file which shows the # open file descriptors in ${valgrind_fds_log}. _val_ensure() { - potential_memleaks_binary=$1 + _val_ensure_potential_memleaks_binary=$1 # Quit if we're not using valgrind. if [ ! "${USE_VALGRIND}" -gt 0 ]; then @@ -191,19 +194,20 @@ _val_ensure() { fi printf "Generating valgrind suppressions... " 1>&2 - valgrind_suppressions_log="${out_valgrind}/suppressions.pre" + _val_ensure_log="${out_valgrind}/suppressions.pre" # Start off with an empty suppression file touch "${valgrind_suppressions}" # Get list of tests and the number of open descriptors at a normal exit - valgrind_suppressions_tests="${out_valgrind}/suppressions-names.txt" + _val_ensure_names="${out_valgrind}/suppressions-names.txt" valgrind --track-fds=yes --log-file="${valgrind_fds_log}" \ - "${potential_memleaks_binary}" > "${valgrind_suppressions_tests}" + "${_val_ensure_potential_memleaks_binary}" \ + > "${_val_ensure_names}" # Generate suppressions for each test - while read -r testname; do - this_valgrind_supp="${valgrind_suppressions_log}-${testname}" + while read -r _val_ensure_testname; do + _val_ensure_thisl="${_val_ensure_log}-${_val_ensure_testname}" # Run valgrind on the binary, sending it a "\n" so that # a test which uses STDIN will not wait for user input. @@ -212,21 +216,22 @@ _val_ensure() { --gen-suppressions=all \ --trace-children=yes \ --suppressions="${valgrind_suppressions}" \ - --log-file="${this_valgrind_supp}" \ - "${potential_memleaks_binary}" \ - "${testname}") \ + --log-file="${_val_ensure_thisl}" \ + "${_val_ensure_potential_memleaks_binary}" \ + "${_val_ensure_testname}") \ > /dev/null # Append name to suppressions file - printf "# %s\n" "${testname}" >> "${valgrind_suppressions}" + printf "# %s\n" "${_val_ensure_testname}" \ + >> "${valgrind_suppressions}" # Strip out useless parts from the log file, and allow the # suppressions to apply to other binaries. - _val_generalize "${this_valgrind_supp}" - done < "${valgrind_suppressions_tests}" + _val_generalize "${_val_ensure_thisl}" + done < "${_val_ensure_names}" # Clean up - rm -f "${valgrind_suppressions_log}" + rm -f "${_val_ensure_log}" printf "done.\n" 1>&2 } @@ -234,7 +239,7 @@ _val_ensure() { # Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to # ${valgrind_min}. If ${str} is not blank, include it in the log filename. valgrind_setup() { - str=${1:-} + _valgrind_setup_str=${1:-} # Bail if we don't want to use valgrind for this check. if [ "${USE_VALGRIND}" -lt "${c_valgrind_min}" ]; then @@ -242,21 +247,21 @@ valgrind_setup() { fi # Set up the log filename. - if [ -n "${str}" ]; then - val_logfilename="${s_val_basename}-${c_count_str}-${str}-%p.log" + if [ -n "${_valgrind_setup_str}" ]; then + _valgrind_setup_logfilename="${s_val_basename}-${c_count_str}-${_valgrind_setup_str}-%p.log" else - val_logfilename="${s_val_basename}-${c_count_str}-%p.log" + _valgrind_setup_logfilename="${s_val_basename}-${c_count_str}-%p.log" fi # Set up valgrind command. - c_valgrind_cmd="valgrind \ - --log-file=${val_logfilename} \ + _valgrind_setup_cmd="valgrind \ + --log-file=${_valgrind_setup_logfilename} \ --track-fds=yes \ --trace-children=yes \ --leak-check=full --show-leak-kinds=all \ --errors-for-leak-kinds=all \ --suppressions=${valgrind_suppressions}" - echo "${c_valgrind_cmd}" + echo "${_valgrind_setup_cmd}" } ## valgrind_incomplete: @@ -275,22 +280,22 @@ valgrind_incomplete() { # Return the filename without ".log" of the valgrind logfile corresponding to # ${exitfile}. _val_getbase() { - exitfile=$1 - basename=$(basename "${exitfile}" ".exit") - echo "${out_valgrind}/${basename}" + _val_getbase_exitfile=$1 + _val_getbase_basename=$(basename "${_val_getbase_exitfile}" ".exit") + echo "${out_valgrind}/${_val_getbase_basename}" } ## _val_checkl(logfile) # Check for any (unsuppressed) memory leaks recorded in a valgrind logfile. # Echo the filename if there's a leak; otherwise, echo nothing. _val_checkl() { - logfile=$1 + _val_checkl_logfile=$1 # Bytes in use at exit. - in_use=$(grep "in use at exit:" "${logfile}" | awk '{print $6}') + _val_checkl_in_use=$(grep "in use at exit:" "${_val_checkl_logfile}" | awk '{print $6}') # Sanity check. - if [ "$(echo "${in_use}" | wc -w)" -ne "1" ]; then + if [ "$(echo "${_val_checkl_in_use}" | wc -w)" -ne "1" ]; then echo "Programmer error: invalid number valgrind outputs" 1>&2 exit 1 fi @@ -298,16 +303,16 @@ _val_checkl() { # Check for any leaks. Use string comparison, because valgrind formats # the number with commas, and sh can't convert strings like "1,000" # into an integer. - if [ "${in_use}" != "0" ] ; then + if [ "${_val_checkl_in_use}" != "0" ] ; then # Check if all of the leaked bytes are suppressed. The extra # whitespace in " suppressed" is necessary to distinguish # between two instances of "suppressed" in the log file. Use # string comparison due to the format of the number. - suppressed=$(grep " suppressed:" "${logfile}" | \ + _val_checkl_suppressed=$(grep " suppressed:" "${_val_checkl_logfile}" | \ awk '{print $3}') - if [ "${in_use}" != "${suppressed}" ]; then + if [ "${_val_checkl_in_use}" != "${_val_checkl_suppressed}" ]; then # There is an unsuppressed leak. - echo "${logfile}" + echo "${_val_checkl_logfile}" return fi fi @@ -319,22 +324,22 @@ _val_checkl() { # descriptors. The important thing is that the number of fds should # match the simple test case (executing potential_memleaks without # running any actual tests). - fds_in_use=$(grep "FILE DESCRIPTORS" "${logfile}" | awk '{print $4}') - valgrind_fds=$(grep "FILE DESCRIPTORS" "${valgrind_fds_log}" | \ + _val_checkl_fds_in_use=$(grep "FILE DESCRIPTORS" "${_val_checkl_logfile}" | awk '{print $4}') + _val_checkl_valgrind_fds=$(grep "FILE DESCRIPTORS" "${valgrind_fds_log}" | \ awk '{print $4}') - if [ "${fds_in_use}" != "${valgrind_fds}" ] ; then + if [ "${_val_checkl_fds_in_use}" != "${_val_checkl_valgrind_fds}" ] ; then # There is an unsuppressed leak. - echo "${logfile}" + echo "${_val_checkl_logfile}" return fi # Check the error summary. - num_errors=$(grep "ERROR SUMMARY: " "${logfile}" | awk '{print $4}') - if [ "${num_errors}" -gt 0 ]; then + _val_checkl_num_errors=$(grep "ERROR SUMMARY: " "${_val_checkl_logfile}" | awk '{print $4}') + if [ "${_val_checkl_num_errors}" -gt 0 ]; then # There was some other error(s) -- invalid read or write, # conditional jump based on uninitialized value(s), invalid # free, etc. - echo "${logfile}" + echo "${_val_checkl_logfile}" return fi } @@ -344,52 +349,52 @@ _val_checkl() { # test exitfile. Return the filename if there's a leak; otherwise return an # empty string. valgrind_check() { - exitfile="$1" - val_basename=$(_val_getbase "${exitfile}") + _valgrind_check_exitfile="$1" + _valgrind_check_basename=$(_val_getbase "$1") # Get list of files to check. (Yes, the star goes outside the quotes.) - logfiles=$(ls "${val_basename}"* 2>/dev/null) - num_logfiles=$(echo "${logfiles}" | wc -w) + _valgrind_check_logfiles=$(ls "${_valgrind_check_basename}"* 2>/dev/null) + _valgrind_check_num=$(echo "${_valgrind_check_logfiles}" | wc -w) # Bail if we don't have any valgrind logfiles to check. # Use numeric comparison, because wc leaves a tab in the output. - if [ "${num_logfiles}" -eq "0" ] ; then + if [ "${_valgrind_check_num}" -eq "0" ] ; then return fi # Check a single file. - if [ "${num_logfiles}" -eq "1" ]; then - _val_checkl "${logfiles}" + if [ "${_valgrind_check_num}" -eq "1" ]; then + _val_checkl "${_valgrind_check_logfiles}" return fi # If the valgrind logfiles contain "-valgrind-parent-", then we only # want to check the parent (the lowest pid). - for logfile in ${logfiles} ; do - if [ "${logfile#*-valgrind-parent-}" != "${logfile}" ]; then + for _valgrind_check_logfile in ${_valgrind_check_logfiles} ; do + if [ "${_valgrind_check_logfile#*-valgrind-parent-}" != "${_valgrind_check_logfile}" ]; then # Only check the parent - _val_checkl "${logfile}" + _val_checkl "${_valgrind_check_logfile}" return "$?" fi done # If there's two files, there's a fork() -- likely within # daemonize() -- so only pay attention to the child. - if [ "${num_logfiles}" -eq "2" ]; then + if [ "${_valgrind_check_num}" -eq "2" ]; then # Find both pids. - val_pids="" - for logfile in ${logfiles} ; do - val_pid=$(head -n 1 "${logfile}" | cut -d "=" -f 3) - val_pids="${val_pids} ${val_pid}" + _valgrind_check_val_pids="" + for _valgrind_check_logfile in ${_valgrind_check_logfiles} ; do + _valgrind_check_val_pid=$(head -n 1 "${_valgrind_check_logfile}" | cut -d "=" -f 3) + _valgrind_check_val_pids="${_valgrind_check_val_pids} ${_valgrind_check_val_pid}" done # Find the logfile which has a parent in the list of pids. - for logfile in ${logfiles} ; do - val_parent_pid=$(grep "Parent PID:" "${logfile}" | \ + for _valgrind_check_logfile in ${_valgrind_check_logfiles} ; do + _valgrind_check_val_parent_pid=$(grep "Parent PID:" "${_valgrind_check_logfile}" | \ awk '{ print $4 }') - if [ "${val_pids#*"${val_parent_pid}"}" != \ - "${val_pids}" ]; then - _val_checkl "${logfile}" + if [ "${_valgrind_check_val_pids#*"${_valgrind_check_val_parent_pid}"}" != \ + "${_valgrind_check_val_pids}" ]; then + _val_checkl "${_valgrind_check_logfile}" return "$?" fi done