On Mon, Jul 01, 2019 at 03:28:45PM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> 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