Re: [PATCH 2/2] test-lib: teach test_seq the -f option
- From
Jeff King <peff@peff.net>
- Date
- Jun 24, 2025, 10:16 UTC
- Message-ID
- <20250624101651.GC636332@coredump.intra.peff.net>
- In-Reply-To
- <aFmOazVXXGpt8bLB@teonanacatl.net>
On Mon, Jun 23, 2025 at 01:27:07PM -0400, Todd Zullinger wrote:
Show 28 quoted lines
> 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.
Show 5 quoted lines
> 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