Re: [PATCH 2/2] test-lib: teach test_seq the -f option

[Jeff King <peff@peff.net>]

On Mon, Jun 23, 2025 at 01:27:07PM -0400, Todd Zullinger wrote:

> Is it a sharp edge worth caring about that someone might
> write `test_seq -f 1 5` where we'd pass 1 as the format
> string?
> 
> If so, perhaps a check like this might be sufficient to
> catch it early?
> 
> 	diff --git i/t/test-lib-functions.sh w/t/test-lib-functions.sh
> 	index 8c176f4efc..87b59d5895 100644
> 	--- i/t/test-lib-functions.sh
> 	+++ w/t/test-lib-functions.sh
> 	@@ -1458,6 +1458,10 @@ test_seq () {
> 		case "$1" in
> 		-f)
> 			fmt="$2"
> 	+		case "$fmt" in
> 	+			*%*)	: ;;
> 	+			*)	BUG "no % in -f argument" ;;
> 	+		esac
> 			shift 2
> 			;;
> 		esac
> 
> I don't know whether it's worth the extra code or not.  I
> just wondered about how it would fail in the face of a minor
> typo.  It certainly should cause any test to fail if it were
> to output 1 instead of the intended format string, so it's
> arguably fine as-is.

Hmm, maybe. I notice that "seq" itself does this (though it did surprise
me). I think there it is actually doing the "%" interpolation itself (to
avoid memory errors by feeding arbitrary strings to printf functions),
so it's easy to do.

In our case, we can rely on the shell printf to do something sensible if
fed garbage. And because we're not parsing ourselves, a pattern like you
have above isn't totally accurate (e.g., consider what it would with
"%%d"). But it probably would be enough to catch typos.

It would also disallow:

  test_seq -f "same line" 50

to produce repeated lines, though I don't know how valuable that would
be. So I dunno.

> Adding -f to the usage note above, as Justin suggested might
> help folks avoid making the mistake of cuddling the format
> string against -f, e.g.: -f%d.  That is caught by the
> parameter count check (though perhaps not everyone would
> notice why, thinking they did pass an argument to -f).

If people are going to use "-f%d", I think we'd be better off making it
work than trying to complain about it. But I was hoping we could just
keep things simple and stupid, given the limited audience.

So my inclination is to leave the sharp edges and see if anybody gets
cut, but it's possible that I'm just being lazy.

-Peff