On Wed, Mar 23 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> Fix two memory leaks in "struct rev_info" by freeing that memory in >> cmd_format_patch(). These two are unusual special-cases in being in >> the "struct rev_info", but not being "owned" by the code in >> revision.c. I.e. they're members of the struct so that this code in >> "builtin/log.c" can pass information code in log-tree.c. > > Hmph, if we are eventually adding a new API function to clear a > rev-info structure, shouldn't these members be released there > instead, I wonder. > > This is the only user of this member, so it does not matter too much > either way, though. Ideally, but in practice "struct rev_info" is both before and after this change a mixed bag of stuff it "owns" and stuff that others "hand over" to it, sometimes permanently, and sometimes just to hold on for a bit. So it can't really free() everything in the struct, at least witohut some larger refactorings, which I'm trying to avoid here. The "diffopt" at the end is another such case, which is left for another follow-up. But for now is it OK if we keep this as-is?
