On Fri, May 31, 2024 at 09:14:49AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

The add_pattern() function takes a pattern string, but neither makes a
copy of it nor takes ownership of the memory. So it is the caller's
responsibility to make sure the string hangs around as long as the
pattern_list which references it.

Wow.  That's an extremely bad API.

I suspect that it long time ago did not aggressively "pre-parse" the
given pattern string and kept the original copy in the struct (i.e.,
taking ownership), and these copies of the literal strings were made
at the calling sites with the expectation that the API takes
ownership of them, as most of the strings fed to add_pattern() are
what we read from the environment into heap.

Yeah, I actually dug into the history a bit but didn't find any real
smoking gun of when it crossed the line to "horrible". It was gradual. ;)

But yes, it was originally just an array of the pattern strings (in
2005!), and then grew more features, and then grew more of an interface
for adding patterns from other sources, and so on. I'm mildly surprised
nobody ran afoul of it before now.

-Peff