Re: [PATCH] test-lib: fix "$remove_trash" regression and match_pattern_list() bugs

On Wed, Jun 16, 2021 at 01:38:55PM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

+	# We need to use "$*" to get field-splitting, but we want to
+	# disable globbing, since we are matching against an arbitrary
+	# $arg, not what's in the filesystem. Using "set -f" accomplishes
+	# that, but we must do it in a subshell to avoid impacting the
+	# rest of the script. The exit value of the subshell becomes
+	# the function's return value.
+	(
+		set -f
+		for pattern_ in $*
+		do
+			case "$arg" in
+			$pattern_)
+				exit 0
+				;;
+			esac
+		done
+		exit 1
+	)
 }

Why not just start with a ret=1, set ret=0 if we have a match and break
from the loop, and then do a "set +f" afterwards? I.e. is there an
actual need for the subshell here.

My thought was that the subshell takes us back to the original state,
regardless of what it was. As opposed to "set +f" which takes us back to
a particular state. But it is unlikely that we'd have done a global "set
-f" before calling this, so maybe that is being overly conservative.

I'm mildly paranoid about a new "set -<flag>" in the codebase for vague
fears of portability (as noted in my linked message), but whatever shell
supports "set -<flag>" surely supports the inverse with "set +<flag>",
no?

Yes, I think we can assume that if it supports one, it supports the
other.

-Peff