On Fri, May 31, 2024 at 09:14:49AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > 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
