Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion scripts/lib/read-list.sh
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,14 @@ read_list::into() {
case "$1" in
--comments)
_rl_mode="${2-}"
shift 2 || true
# Shift only what is actually there. `shift 2` with `--comments` as the
# LAST argument shifts nothing and returns non-zero, and the `|| true`
# this replaces swallowed that: `$#` stayed at 1 and the loop reprocessed
# `--comments` forever (#3363) instead of ever reaching the rc-2 branch
# below. Draining to `$# == 0` lets a bare `--comments` fall through to
# the existing "mode is required" error, the same answer `--comments ''`
# already gave.
shift $(($# > 1 ? 2 : 1))
;;
*)
printf 'read-list: unknown option %s\n' "$1" >&2
Expand Down
47 changes: 47 additions & 0 deletions scripts/lib/read-list.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,53 @@ if read_list::into entries "$f" --bogus 2>/dev/null; then
else
ok "an unknown option is rejected"
fi

# A BARE `--comments` (the flag present, its mode value missing) must reach the
# same rc-2 answer, and must reach it AT ALL. The pre-#3363 arm consumed its
# value with `shift 2 || true`; as the last argument that shifted nothing and
# returned non-zero, `|| true` hid the failure, `$#` stayed at 1, and the option
# loop reprocessed `--comments` forever.
#
# The WATCHDOG is the load-bearing part of this assertion, not decoration:
# without a bound, a regression does not FAIL this suite, it HANGS it, and an
# unbounded stall in CI is worse than a red test. It is hand-rolled from
# `kill`/`wait` rather than written as `timeout 5` because GNU coreutils
# `timeout` is absent from a stock macOS userland, which
# scripts/check-shell-portability.sh names as the platform no runner here
# covers: on a developer's Mac that invocation would return 127 and fail a
# correct library. `sleep`, `kill` and `wait` are POSIX, so this bounds the
# probe everywhere. A killed probe reports 128+SIGKILL, never 2.
missing_value_rc=0
# shellcheck disable=SC2016 # $1/$2 are the inner `bash -c` positionals, deliberately unexpanded here
bash -c '
. "$1"
declare -a probe=()
read_list::into probe "$2" --comments
' _ "$SELF_DIR/read-list.sh" "$f" 2>/dev/null &
probe_pid=$!
# SIGKILL on the watchdog SHELL, and no `wait` for it. SIGTERM would be
# deferred until its foreground `sleep` returned, costing the suite the full
# five seconds on the HAPPY path; SIGKILL cannot be deferred. Killing the shell
# rather than letting it fire against an already-reaped pid is also what keeps
# this free of a pid-reuse hazard: the orphaned `sleep` has nothing left to run
# the kill.
# The `>/dev/null 2>&1` is not cosmetic. Without it the orphaned `sleep`
# inherits this suite's stdout, and anything reading the suite through a pipe
# blocks for the full five seconds waiting for that last writer to close.
(
sleep 5
kill -9 "$probe_pid" 2>/dev/null
) >/dev/null 2>&1 &
watchdog_pid=$!
wait "$probe_pid" || missing_value_rc=$?
kill -9 "$watchdog_pid" 2>/dev/null
if [[ "$missing_value_rc" -eq 2 ]]; then
ok "a bare --comments (no mode value) returns 2 promptly"
elif [[ "$missing_value_rc" -ge 128 ]]; then
fail "a bare --comments HUNG (watchdog killed it, rc=$missing_value_rc) instead of returning 2 (#3363)"
else
fail "a bare --comments returned $missing_value_rc, want 2"
fi
rm -f "$f"

# --- an unreadable file is loud, never an empty list -----------------------
Expand Down