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

From: Jeff King <peff@peff.net>
Date: Jul 2, 2019, 05:22 UTC
Message-ID: <20190702052243.GB16344@sigill.intra.peff.net>
In-Reply-To: <xmqq36jp5qzm.fsf@gitster-ct.c.googlers.com>

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

Show 8 quoted lines

> 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