On Fri, Jul 29, 2022 at 11:52:02AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

The more interesting question is whether it causes any use-after-free
bugs.

Thanks for mentioning this.  All the "plug more leaks" patches make
me worried for exactly that.  Another is a potential subtle breakage
hidden by use of FREE_AND_NULL() and friends, which the sanitizers
would probably not see, but can appear as behaviour change.

Yeah, agreed that is a potential pitfall.

quoted

I don't think it does, and certainly SANITIZE=address agrees.

;-)

Just to expound a bit: I'm quite sure _this_ call site is fine, because
we are passing pretty vanilla options to setup_revisions(), and I looked
at how we handle the strings there. I'd worry a little more that there's
some dark corner of setup_revisions() which relies on memory lifetimes,
and so this isn't a safe thing to do in the general case.

But IMHO we should assume that setup_revisions() makes copies of strings
it needs, and if there is a dark corner where it doesn't, we should find
and fix that.

-Peff