From 53f5b23f3c1e9c5b4c07131521131af7df9b3b42 Mon Sep 17 00:00:00 2001
From: Graham Percival <gperciva@tarsnap.com>
Date: Wed, 30 Aug 2023 10:12:29 -0700
Subject: [PATCH 1/6] scripts: fix comments

- Add find_system() to the "public API", which should have been part of:
    2023-06-29 scripts: misc fixes
    ed4c19ad5a55963e69792209a1eafc12f14c068d
  find_system() is not used by any other scripts in libcperciva, but
  other projects make use of it.

- Fix the API comment for run_scenarios(), which should have been part of:
    2021-03-24 tests: add `make test N=` to shared framework
    acea39b8aeaeb88a8fa86edb1e8e7c59c200585f

- Fix the API comment for valgrind_check_optional(), which was incorrect
  from the very beginning:
    2017-10-28 tests: import shared_test_functions.sh
    d486415352a2c95a98cc4935fd4ca6861ff8ed9c

- Use {} for $USE_VALGRIND and $PATH in comments.  This should have been
  part of:
    2023-08-06 scripts: always use double quotes and curly braces
    a1e8ac785558b317de84d3821be27fa525d359ef
---
 tests/shared_test_functions.sh     |  9 ++++++---
 tests/shared_valgrind_functions.sh | 10 +++++-----
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tests/shared_test_functions.sh b/tests/shared_test_functions.sh
index 99924060..4ca19716 100644
--- a/tests/shared_test_functions.sh
+++ b/tests/shared_test_functions.sh
@@ -18,6 +18,8 @@
 # function which was defined in that file.
 #
 # Functions which are available to other scripts as a "public API" are:
+# - find_system(cmd, args):
+#   Look for ${cmd} in the ${PATH}, and ensure that it supports ${args}.
 # - has_pid(cmd):
 #   Look for a ${cmd} in $(ps).
 # - wait_while(func):
@@ -86,7 +88,7 @@ prepare_directory() {
 }
 
 ## find_system (cmd, args):
-# Look for ${cmd} in the $PATH, and ensure that it supports ${args}.
+# Look for ${cmd} in the ${PATH}, and ensure that it supports ${args}.
 find_system() {
 	cmd=$1
 	cmd_with_args="$1 ${2:-}"
@@ -312,8 +314,9 @@ scenario_runner() {
 	return "${s_retval}"
 }
 
-## run_scenarios (scenario_filenames):
-# Run all scenarios matching ${scenario_filenames}.
+## run_scenarios ():
+# Run all scenarios in the test directory.  If the environment variable ${N}
+# is specified, only run the scenario corresponding to that number.
 run_scenarios() {
 	# Get the test number(s) to run.
 	if [ "${N:-0}" -gt "0" ]; then
diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh
index 0d8e5b99..616ffb63 100644
--- a/tests/shared_valgrind_functions.sh
+++ b/tests/shared_valgrind_functions.sh
@@ -11,7 +11,7 @@ set -o noclobber -o nounset
 #   Clear previous valgrind output, and prepare for running valgrind tests
 #   (if applicable).
 # - valgrind_setup_cmd():
-#   Set up the valgrind command if $USE_VALGRIND is greater than or equal to
+#   Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to
 #   ${valgrind_min}.
 # - valgrind_check_basenames(exitfile):
 #   Check for any memory leaks recorded in valgrind logfiles associated with a
@@ -69,11 +69,11 @@ valgrind_prepare_directory() {
 }
 
 ## valgrind_check_optional ():
-# 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.
+# 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() {
 	if [ "${USE_VALGRIND}" -gt 0 ]; then
-		# Look for valgrind in $PATH.
+		# Look for valgrind in ${PATH}.
 		if ! command -v valgrind >/dev/null 2>&1; then
 			printf "valgrind not found\n" 1>&2
 			exit 1
@@ -222,7 +222,7 @@ valgrind_ensure_suppression() {
 }
 
 ## valgrind_setup_cmd ():
-# Set up the valgrind command if $USE_VALGRIND is greater than or equal to
+# Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to
 # ${valgrind_min}.
 valgrind_setup_cmd() {
 	# Bail if we don't want to use valgrind for this check.

From 7cabc61ef1eb3ee70ed4f35b8717f688e5540842 Mon Sep 17 00:00:00 2001
From: Graham Percival <gperciva@tarsnap.com>
Date: Wed, 30 Aug 2023 10:12:30 -0700
Subject: [PATCH 2/6] scripts: clarify public variables in valgrind

- document the public variables
- define them in a single place
- instead of recording the number of open file descriptors (as a public
  variable), just keep the relevant filename; we can extract the actual
  number every time we need it.  This simplifies the logic of
  USE_VALGRIND_NO_REGEN.
---
 tests/shared_valgrind_functions.sh | 31 +++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh
index 616ffb63..0a25f337 100644
--- a/tests/shared_valgrind_functions.sh
+++ b/tests/shared_valgrind_functions.sh
@@ -19,6 +19,13 @@ set -o noclobber -o nounset
 #   empty string.
 # - valgrind_incomplete():
 #   Check if any valgrind log files are incomplete.
+#
+### Variables
+#
+# Wherever possible, this suite uses local variables and
+# explicitly-passed arguments, with the following exceptions:
+# - 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.
@@ -30,8 +37,6 @@ valgrind_exit_code=108
 valgrind_prepare_directory() {
 	# If we don't want to generate new suppressions files, move them.
 	if [ "${USE_VALGRIND_NO_REGEN}" -gt 0 ]; then
-		valgrind_suppressions="${out_valgrind}/suppressions"
-		fds="${out_valgrind}/fds.log"
 		# Bail if the file doesn't exist.
 		if [ ! -e "${valgrind_suppressions}" ]; then
 			echo "No valgrind suppressions file" 1>&2
@@ -42,7 +47,7 @@ valgrind_prepare_directory() {
 		supp_tmp="$(mktemp /tmp/valgrind-suppressions.XXXXXX)"
 		fds_tmp="$(mktemp /tmp/valgrind-fds.XXXXXX)"
 		mv "${valgrind_suppressions}" "${supp_tmp}"
-		mv "${fds}" "${fds_tmp}"
+		mv "${valgrind_fds_log}" "${fds_tmp}"
 	fi
 
 	# Always delete any previous valgrind directory.
@@ -61,7 +66,7 @@ valgrind_prepare_directory() {
 	if [ "${USE_VALGRIND_NO_REGEN}" -gt 0 ]; then
 		# Move the files back.
 		mv "${supp_tmp}" "${valgrind_suppressions}"
-		mv "${fds_tmp}" "${fds}"
+		mv "${fds_tmp}" "${valgrind_fds_log}"
 	fi
 
 	# We don't want to back up this directory.
@@ -161,8 +166,8 @@ valgrind_process_suppression_file() {
 ## valgrind_ensure_suppression (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 how many file descriptors
-# are open at exit in ${valgrind_fds}.
+# those leaks when testing other binaries.  Record a log file which shows the
+# open file descriptors in ${valgrind_fds_log}.
 valgrind_ensure_suppression() {
 	potential_memleaks_binary=$1
 
@@ -171,17 +176,12 @@ valgrind_ensure_suppression() {
 		return
 	fi;
 
-	fds_log="${out_valgrind}/fds.log"
-
 	if [ "${USE_VALGRIND_NO_REGEN}" -gt 0 ]; then
 		printf "Using old valgrind suppressions\n" 1>&2
-		valgrind_fds=$(grep "FILE DESCRIPTORS" "${fds_log}" |	\
-		   awk '{print $4}')
 		return
 	fi
 
 	printf "Generating valgrind suppressions... " 1>&2
-	valgrind_suppressions="${out_valgrind}/suppressions"
 	valgrind_suppressions_log="${out_valgrind}/suppressions.pre"
 
 	# Start off with an empty suppression file
@@ -189,9 +189,8 @@ valgrind_ensure_suppression() {
 
 	# Get list of tests and the number of open descriptors at a normal exit
 	valgrind_suppressions_tests="${out_valgrind}/suppressions-names.txt"
-	valgrind --track-fds=yes --log-file="${fds_log}"		\
+	valgrind --track-fds=yes --log-file="${valgrind_fds_log}"	\
 	    "${potential_memleaks_binary}" > "${valgrind_suppressions_tests}"
-	valgrind_fds=$(grep "FILE DESCRIPTORS" "${fds_log}" | awk '{print $4}')
 
 	# Generate suppressions for each test
 	while read -r testname; do
@@ -301,6 +300,8 @@ valgrind_check_logfile() {
 	# 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}" | \
+	    awk '{print $4}')
 	if [ "${fds_in_use}" != "${valgrind_fds}" ] ; then
 		# There is an unsuppressed leak.
 		echo "${logfile}"
@@ -373,6 +374,10 @@ valgrind_check_basenames() {
 # Clear previous valgrind output, and prepare for running valgrind tests
 # (if applicable).
 valgrind_init() {
+	# Set up global variables.
+	valgrind_suppressions="${out_valgrind}/suppressions"
+	valgrind_fds_log="${out_valgrind}/fds.log"
+
 	# If we want valgrind, check that the version is high enough.
 	valgrind_check_optional
 

From 4f3c0a524b574db75015dc7d5987ca11a41945bc Mon Sep 17 00:00:00 2001
From: Graham Percival <gperciva@tarsnap.com>
Date: Wed, 30 Aug 2023 10:12:31 -0700
Subject: [PATCH 3/6] scripts: use underscores for "private" functions

In POSIX sh, there's no such thing as "public" or "private" functions.

However, functions are allowed to contain underscores [1], so we're
adopting the convention of leading underscores indicating a function
which is not intended to be called by code in a different file.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_235

Private functions in shared_test_functions.sh:
    prepare_directory
    notify_success_or_fail
    scenario_runner

Private functions in shared_valgrind_functions.sh:
    valgrind_prepare_directory
    valgrind_check_optional
    valgrind_process_suppression_file
    valgrind_ensure_suppression
    valgrind_get_basename
    valgrind_check_logfile
---
 tests/shared_test_functions.sh     | 22 ++++++++--------
 tests/shared_valgrind_functions.sh | 40 ++++++++++++++++--------------
 2 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/tests/shared_test_functions.sh b/tests/shared_test_functions.sh
index 4ca19716..ffedc094 100644
--- a/tests/shared_test_functions.sh
+++ b/tests/shared_test_functions.sh
@@ -12,7 +12,7 @@
 #
 ### Design
 #
-# The main function is scenario_runner(scenario_filename), which
+# The main function is _scenario_runner(scenario_filename), which
 # takes a scenario file as the argument, and runs a
 #     scenario_cmd()
 # function which was defined in that file.
@@ -31,6 +31,8 @@
 # - run_scenarios():
 #   Run scenarios in the test directory.
 #
+# We adopt the convention of "private" function names beginning with an _.
+#
 ### Variables
 #
 # Wherever possible, this suite uses local variables and
@@ -75,9 +77,9 @@ bindir=$(CDPATH='' cd -- "$(dirname -- "${1-.}")" && pwd -P)
 NO_EXITFILE=/dev/null
 
 
-## prepare_directory():
+## _prepare_directory():
 # Delete the previous test output directory, and create a new one.
-prepare_directory() {
+_prepare_directory() {
 	if [ -d "${out}" ]; then
 		rm -rf "${out}"
 	fi
@@ -209,14 +211,14 @@ expected_exitcode() {
 	fi
 }
 
-## notify_success_or_fail (log_basename, val_log_basename):
+## _notify_success_or_fail (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() {
+_notify_success_or_fail() {
 	log_basename=$1
 	val_log_basename=$2
 
@@ -280,9 +282,9 @@ notify_success_or_fail() {
 	fi
 }
 
-## scenario_runner (scenario_filename):
+## _scenario_runner (scenario_filename):
 # Run a test scenario from ${scenario_filename}.
-scenario_runner() {
+_scenario_runner() {
 	scenario_filename=$1
 	basename=$(basename "${scenario_filename}" .sh)
 	printf "  %s... " "${basename}" 1>&2
@@ -309,7 +311,7 @@ scenario_runner() {
 
 	# Print PASS or FAIL, and return result.
 	s_retval=0
-	notify_success_or_fail "${s_basename}" "${s_val_basename}"
+	_notify_success_or_fail "${s_basename}" "${s_val_basename}"
 
 	return "${s_retval}"
 }
@@ -326,7 +328,7 @@ run_scenarios() {
 	fi
 
 	# Clean up any previous directory, and create a new one.
-	prepare_directory
+	_prepare_directory
 
 	# Clean up any previous valgrind directory, and prepare for new
 	# valgrind tests (if applicable).
@@ -337,7 +339,7 @@ run_scenarios() {
 	for scenario in ${test_scenarios}; do
 		# We can't call this function with $( ... ) because we
 		# want to allow it to echo values to stdout.
-		scenario_runner "${scenario}"
+		_scenario_runner "${scenario}"
 		retval=$?
 		if [ "${retval}" -gt 0 ]; then
 			exit "${retval}"
diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh
index 0a25f337..bf633123 100644
--- a/tests/shared_valgrind_functions.sh
+++ b/tests/shared_valgrind_functions.sh
@@ -20,6 +20,8 @@ set -o noclobber -o nounset
 # - valgrind_incomplete():
 #   Check if any valgrind log files are incomplete.
 #
+# We adopt the convention of "private" function names beginning with an _.
+#
 ### Variables
 #
 # Wherever possible, this suite uses local variables and
@@ -31,10 +33,10 @@ set -o noclobber -o nounset
 # tested.
 valgrind_exit_code=108
 
-## valgrind_prepare_directory ():
+## _valgrind_prepare_directory ():
 # Clean up a previous valgrind directory, and prepare for new valgrind tests
 # (if applicable).
-valgrind_prepare_directory() {
+_valgrind_prepare_directory() {
 	# 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.
@@ -73,10 +75,10 @@ valgrind_prepare_directory() {
 	[ "$(uname)" = "FreeBSD" ] && chflags nodump "${out_valgrind}"
 }
 
-## valgrind_check_optional ():
+## _valgrind_check_optional ():
 # 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() {
+_valgrind_check_optional() {
 	if [ "${USE_VALGRIND}" -gt 0 ]; then
 		# Look for valgrind in ${PATH}.
 		if ! command -v valgrind >/dev/null 2>&1; then
@@ -99,10 +101,10 @@ valgrind_check_optional() {
 	fi
 }
 
-## valgrind_process_suppression_file(filename):
+## _valgrind_process_suppression_file(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() {
+_valgrind_process_suppression_file() {
 	filename=$1
 
 	# How many segments do we have?
@@ -163,12 +165,12 @@ valgrind_process_suppression_file() {
 	done
 }
 
-## valgrind_ensure_suppression (potential_memleaks_binary):
+## _valgrind_ensure_suppression (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() {
+_valgrind_ensure_suppression() {
 	potential_memleaks_binary=$1
 
 	# Quit if we're not using valgrind.
@@ -212,7 +214,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}"
+		_valgrind_process_suppression_file "${this_valgrind_supp}"
 	done < "${valgrind_suppressions_tests}"
 
 	# Clean up
@@ -251,19 +253,19 @@ valgrind_incomplete() {
 	test -n "${_valgrind_incomplete_logfiles}"
 }
 
-## valgrind_get_basename (exitfile):
+## _valgrind_get_basename (exitfile):
 # Return the filename without ".log" of the valgrind logfile corresponding to
 # ${exitfile}.
-valgrind_get_basename() {
+_valgrind_get_basename() {
 	exitfile=$1
 	basename=$(basename "${exitfile}" ".exit")
 	echo "${out_valgrind}/${basename}"
 }
 
-## valgrind_check_logfile(logfile)
+## _valgrind_check_logfile(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() {
+_valgrind_check_logfile() {
 	logfile=$1
 
 	# Bytes in use at exit.
@@ -325,7 +327,7 @@ valgrind_check_logfile() {
 # empty string.
 valgrind_check_basenames() {
 	exitfile="$1"
-	val_basename=$(valgrind_get_basename "${exitfile}")
+	val_basename=$(_valgrind_get_basename "${exitfile}")
 
 	# Get list of files to check.  (Yes, the star goes outside the quotes.)
 	logfiles=$(ls "${val_basename}"* 2>/dev/null)
@@ -339,7 +341,7 @@ valgrind_check_basenames() {
 
 	# Check a single file.
 	if [ "${num_logfiles}" -eq "1" ]; then
-		valgrind_check_logfile "${logfiles}"
+		_valgrind_check_logfile "${logfiles}"
 		return
 	fi
 
@@ -359,7 +361,7 @@ valgrind_check_basenames() {
 			    awk '{ print $4 }')
 			if [ "${val_pids#*"${val_parent_pid}"}" !=	\
 			    "${val_pids}" ]; then
-				valgrind_check_logfile "${logfile}"
+				_valgrind_check_logfile "${logfile}"
 				return "$?"
 			fi
 		done
@@ -379,13 +381,13 @@ valgrind_init() {
 	valgrind_fds_log="${out_valgrind}/fds.log"
 
 	# If we want valgrind, check that the version is high enough.
-	valgrind_check_optional
+	_valgrind_check_optional
 
 	# Remove any previous directory, and create a new one.
-	valgrind_prepare_directory
+	_valgrind_prepare_directory
 
 	# Generate valgrind suppression file if it is required.  Must be
 	# done after preparing the directory.
-	valgrind_ensure_suppression				\
+	_valgrind_ensure_suppression				\
 	    "${bindir}/tests/valgrind/potential-memleaks"
 }

From 0846c859a00a3512e0e22ff4c2e7eb15317f1bf9 Mon Sep 17 00:00:00 2001
From: Graham Percival <gperciva@tarsnap.com>
Date: Wed, 20 Sep 2023 09:41:48 -0700
Subject: [PATCH 4/6] Trivial comments

- Fix single space after period
- Add "Success!" where appropriate
---
 lib/scryptenc/scryptenc_cpuperf.c                    | 1 +
 libcperciva/POSIX/posix-abstract-declarator.c        | 4 ++++
 libcperciva/POSIX/posix-stat-st_mtim.c               | 1 +
 libcperciva/POSIX/posix-trivial.c                    | 1 +
 libcperciva/cpusupport/Build/cpusupport-ARM-AES.c    | 1 +
 libcperciva/cpusupport/Build/cpusupport-ARM-SHA256.c | 1 +
 libcperciva/crypto/crypto_aesctr_shared.c            | 1 +
 libcperciva/util/monoclock.c                         | 4 ++--
 libcperciva/util/monoclock.h                         | 4 ++--
 9 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/lib/scryptenc/scryptenc_cpuperf.c b/lib/scryptenc/scryptenc_cpuperf.c
index 0836dc13..b7a64fb9 100644
--- a/lib/scryptenc/scryptenc_cpuperf.c
+++ b/lib/scryptenc/scryptenc_cpuperf.c
@@ -46,6 +46,7 @@ getclockdiff(struct timeval * st, double * diffd)
 		return (1);
 	*diffd = timeval_diff((*st), en);
 
+	/* Success! */
 	return (0);
 }
 
diff --git a/libcperciva/POSIX/posix-abstract-declarator.c b/libcperciva/POSIX/posix-abstract-declarator.c
index c4ec2498..40e71b2c 100644
--- a/libcperciva/POSIX/posix-abstract-declarator.c
+++ b/libcperciva/POSIX/posix-abstract-declarator.c
@@ -9,6 +9,8 @@ func(int arr[static restrict 1])
 {
 
 	(void)arr; /* UNUSED */
+
+	/* Success! */
 	return (0);
 }
 
@@ -17,5 +19,7 @@ main(void)
 {
 
 	(void)func; /* UNUSED */
+
+	/* Success! */
 	return (0);
 }
diff --git a/libcperciva/POSIX/posix-stat-st_mtim.c b/libcperciva/POSIX/posix-stat-st_mtim.c
index 7e861e22..73d5dcc9 100644
--- a/libcperciva/POSIX/posix-stat-st_mtim.c
+++ b/libcperciva/POSIX/posix-stat-st_mtim.c
@@ -8,5 +8,6 @@ main(void)
 	/* Can we reference st_mtim? */
 	(void)sb.st_mtim.tv_sec;
 
+	/* Success! */
 	return (0);
 }
diff --git a/libcperciva/POSIX/posix-trivial.c b/libcperciva/POSIX/posix-trivial.c
index 9f534cdf..5036a736 100644
--- a/libcperciva/POSIX/posix-trivial.c
+++ b/libcperciva/POSIX/posix-trivial.c
@@ -2,5 +2,6 @@ int
 main(void)
 {
 
+	/* Success! */
 	return (0);
 }
diff --git a/libcperciva/cpusupport/Build/cpusupport-ARM-AES.c b/libcperciva/cpusupport/Build/cpusupport-ARM-AES.c
index a3f7b5b8..68900697 100644
--- a/libcperciva/cpusupport/Build/cpusupport-ARM-AES.c
+++ b/libcperciva/cpusupport/Build/cpusupport-ARM-AES.c
@@ -17,5 +17,6 @@ main(void)
 
 	lanes = vdupq_laneq_u32(lanes, 0);
 
+	/* Success! */
 	return (0);
 }
diff --git a/libcperciva/cpusupport/Build/cpusupport-ARM-SHA256.c b/libcperciva/cpusupport/Build/cpusupport-ARM-SHA256.c
index 2768f618..7a04d87b 100644
--- a/libcperciva/cpusupport/Build/cpusupport-ARM-SHA256.c
+++ b/libcperciva/cpusupport/Build/cpusupport-ARM-SHA256.c
@@ -12,5 +12,6 @@ main(void)
 	output = vsha256su0q_u32(w0, w4);
 	(void)output; /* UNUSED */
 
+	/* Success! */
 	return (0);
 }
diff --git a/libcperciva/crypto/crypto_aesctr_shared.c b/libcperciva/crypto/crypto_aesctr_shared.c
index 531bbeb8..632e419c 100644
--- a/libcperciva/crypto/crypto_aesctr_shared.c
+++ b/libcperciva/crypto/crypto_aesctr_shared.c
@@ -81,6 +81,7 @@ crypto_aesctr_stream_pre_wholeblock(struct crypto_aesctr * stream,
 		    buflen_p, 16 - bytemod, bytemod);
 	}
 
+	/* Success! */
 	return (0);
 }
 
diff --git a/libcperciva/util/monoclock.c b/libcperciva/util/monoclock.c
index 5dfc0150..4cd82f4c 100644
--- a/libcperciva/util/monoclock.c
+++ b/libcperciva/util/monoclock.c
@@ -97,8 +97,8 @@ monoclock_get_cputime(struct timeval * tv)
 
 /**
  * monoclock_getres(resd):
- * Store an upper limit on timer granularity in ${resd}. If CLOCK_MONOTONIC is
- * available, use that clock; if CLOCK_MONOTONIC is unavailable, use
+ * Store an upper limit on timer granularity in ${resd}.  If CLOCK_MONOTONIC
+ * is available, use that clock; if CLOCK_MONOTONIC is unavailable, use
  * CLOCK_REALTIME (if available) or gettimeofday(2).  For this value to be
  * meaningful, we assume that clock_getres(x) succeeds iff clock_gettime(x)
  * succeeds.
diff --git a/libcperciva/util/monoclock.h b/libcperciva/util/monoclock.h
index 310c4467..ff0cded7 100644
--- a/libcperciva/util/monoclock.h
+++ b/libcperciva/util/monoclock.h
@@ -25,8 +25,8 @@ int monoclock_get_cputime(struct timeval *);
 
 /**
  * monoclock_getres(resd):
- * Store an upper limit on timer granularity in ${resd}. If CLOCK_MONOTONIC is
- * available, use that clock; if CLOCK_MONOTONIC is unavailable, use
+ * Store an upper limit on timer granularity in ${resd}.  If CLOCK_MONOTONIC
+ * is available, use that clock; if CLOCK_MONOTONIC is unavailable, use
  * CLOCK_REALTIME (if available) or gettimeofday(2).  For this value to be
  * meaningful, we assume that clock_getres(x) succeeds iff clock_gettime(x)
  * succeeds.

From 6d91a80cad184db6d11c3fcdb2c9550c294cedb3 Mon Sep 17 00:00:00 2001
From: Graham Percival <gperciva@tarsnap.com>
Date: Wed, 20 Sep 2023 09:41:49 -0700
Subject: [PATCH 5/6] tests/valgrind: improve handling for fork() and exec()

- use trace-children=yes.  (This actually means "trace exec"; the
  valgrind man page even admits that the option is badly named!)

- allow our tests to only check the parent pid

In more detail, in our tests/??-*.sh, we would set up:

    scenario_cmd() {
        ...
        c_valgrind_cmd=$(valgrind_setup_cmd "valgrind-parent")
        ${c_valgrind_cmd} ./test_something_with_forks

If we have two pids from a single binary, then our valgrind test will
assume that we're using daemonize() and ignore any leaks from the parent
pid.  (This is exactly what we want for daemonize().)

However, if we use fork() but keep the parent, then we want the
opposite: we should check for leaks in the parent, and ignore leaks from
the children.

We need to explicitly add something to the log filename beacuse if we
didn't, then a valgrind check with two pids wouldn't know whether to
complain about leaks in the parent or child.
---
 tests/shared_valgrind_functions.sh | 31 +++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/tests/shared_valgrind_functions.sh b/tests/shared_valgrind_functions.sh
index bf633123..83ad7f35 100644
--- a/tests/shared_valgrind_functions.sh
+++ b/tests/shared_valgrind_functions.sh
@@ -10,9 +10,9 @@ set -o noclobber -o nounset
 # - valgrind_init():
 #   Clear previous valgrind output, and prepare for running valgrind tests
 #   (if applicable).
-# - valgrind_setup_cmd():
+# - valgrind_setup_cmd(str):
 #   Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to
-#   ${valgrind_min}.
+#   ${valgrind_min}.  If ${str} is not blank, include it in the log filename.
 # - valgrind_check_basenames(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
@@ -203,6 +203,7 @@ _valgrind_ensure_suppression() {
 		printf "\n" | (valgrind					\
 		    --leak-check=full --show-leak-kinds=all		\
 		    --gen-suppressions=all				\
+		    --trace-children=yes				\
 		    --suppressions="${valgrind_suppressions}"		\
 		    --log-file="${this_valgrind_supp}"			\
 		    "${potential_memleaks_binary}"			\
@@ -222,19 +223,29 @@ _valgrind_ensure_suppression() {
 	printf "done.\n" 1>&2
 }
 
-## valgrind_setup_cmd ():
+## valgrind_setup_cmd (str):
 # Set up the valgrind command if ${USE_VALGRIND} is greater than or equal to
-# ${valgrind_min}.
+# ${valgrind_min}.  If ${str} is not blank, include it in the log filename.
 valgrind_setup_cmd() {
+	str=${1:-}
+
 	# Bail if we don't want to use valgrind for this check.
 	if [ "${USE_VALGRIND}" -lt "${c_valgrind_min}" ]; then
 		return
 	fi
 
-	val_logfilename="${s_val_basename}-${c_count_str}-%p.log"
+	# Set up the log filename.
+	if [ -n "${str}" ]; then
+		val_logfilename="${s_val_basename}-${c_count_str}-${str}-%p.log"
+	else
+		val_logfilename="${s_val_basename}-${c_count_str}-%p.log"
+	fi
+
+	# Set up valgrind command.
 	c_valgrind_cmd="valgrind \
 		--log-file=${val_logfilename} \
 		--track-fds=yes \
+		--trace-children=yes \
 		--leak-check=full --show-leak-kinds=all \
 		--errors-for-leak-kinds=all \
 		--suppressions=${valgrind_suppressions}"
@@ -345,6 +356,16 @@ valgrind_check_basenames() {
 		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
+			# Only check the parent
+			_valgrind_check_logfile "${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

From 442cf004a702b4a783fa922e2531da02a3588358 Mon Sep 17 00:00:00 2001
From: Graham Percival <gperciva@tarsnap.com>
Date: Mon, 25 Sep 2023 18:54:25 -0700
Subject: [PATCH 6/6] style: fix function pointers

- fix missing space between return type and opening parenthesis
- fix missing space after * or **
---
 lib-platform/crypto/crypto_scrypt.c | 6 +++---
 libcperciva/alg/sha256.c            | 2 +-
 libcperciva/crypto/crypto_aes.c     | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib-platform/crypto/crypto_scrypt.c b/lib-platform/crypto/crypto_scrypt.c
index a98d4306..6057f0af 100644
--- a/lib-platform/crypto/crypto_scrypt.c
+++ b/lib-platform/crypto/crypto_scrypt.c
@@ -45,7 +45,7 @@
 
 #include "crypto_scrypt.h"
 
-static void (*smix_func)(uint8_t *, size_t, uint64_t, void *, void *) = NULL;
+static void (* smix_func)(uint8_t *, size_t, uint64_t, void *, void *) = NULL;
 
 /**
  * crypto_scrypt_internal(passwd, passwdlen, salt, saltlen, N, r, p, buf,
@@ -56,7 +56,7 @@ static int
 crypto_scrypt_internal(const uint8_t * passwd, size_t passwdlen,
     const uint8_t * salt, size_t saltlen, uint64_t N, uint32_t _r, uint32_t _p,
     uint8_t * buf, size_t buflen,
-    void (*smix)(uint8_t *, size_t, uint64_t, void *, void *))
+    void (* smix)(uint8_t *, size_t, uint64_t, void *, void *))
 {
 	void * B0, * V0, * XY0;
 	uint8_t * B;
@@ -192,7 +192,7 @@ static struct scrypt_test {
 };
 
 static int
-testsmix(void (*smix)(uint8_t *, size_t, uint64_t, void *, void *))
+testsmix(void (* smix)(uint8_t *, size_t, uint64_t, void *, void *))
 {
 	uint8_t hbuf[TESTLEN];
 
diff --git a/libcperciva/alg/sha256.c b/libcperciva/alg/sha256.c
index d698eddc..1321f1f0 100644
--- a/libcperciva/alg/sha256.c
+++ b/libcperciva/alg/sha256.c
@@ -140,7 +140,7 @@ static int
 hwtest(const uint32_t state[static restrict 8],
     const uint8_t block[static restrict 64],
     uint32_t W[static restrict 64], uint32_t S[static restrict 8],
-    void(* func)(uint32_t state[static restrict 8],
+    void (* func)(uint32_t state[static restrict 8],
     const uint8_t block[static restrict 64], uint32_t W[static restrict 64],
     uint32_t S[static restrict 8]))
 {
diff --git a/libcperciva/crypto/crypto_aes.c b/libcperciva/crypto/crypto_aes.c
index 8d64761c..f884605b 100644
--- a/libcperciva/crypto/crypto_aes.c
+++ b/libcperciva/crypto/crypto_aes.c
@@ -67,7 +67,7 @@ static struct aes_test {
 
 /* Test a function against test vectors. */
 static int
-functest(int(* func)(const uint8_t *, size_t, const uint8_t[16], uint8_t[16]))
+functest(int (* func)(const uint8_t *, size_t, const uint8_t[16], uint8_t[16]))
 {
 	struct aes_test * knowngood;
 	uint8_t ctext[16];