Re: [PATCH v2 1/6] test-lib: introduce test_commit_bulk

[Jeff King <peff@peff.net>]

On Mon, Jul 01, 2019 at 03:28:45PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >   - replace eval formatting magic with "%s" printf formatters (safer and
> >     gets rid of quoting issues in the callers).
> 
> This one actually made me think twice about safety, as we'd be using
> end-user supplied formatting string without any inspection.  I think
> it is fine as it is merely a test helper.

Yeah, and most shells do something sensible with nonsense formats.
E.g., "%s %s" will yield an empty string for the second one in both dash
and bash (and that's what POSIX says, though I'd be happy with any
implementation that avoids segfaulting).

> If somebody is later making it into a test-tool function, I expect
> that our interpolation engine, not the bare sprintf(), would be used
> there, and it would hopefully also be safe?

Yes, that was exactly my plan. It would also let you mention the number
more than once in the format, though I doubt any callers would care
about that feature.

I also think more potential callers could be converted if the refname
was formatted, too (e.g., some of them seem to write to branch-1,
branch-2, etc). I drew the line there, but anybody is welcome to explore
it further.

-Peff