Junio C Hamano <gitster@xxxxxxxxx> 于2021年4月21日周三 上午2:00写道: > > René Scharfe. <l.s.r@xxxxxx> writes: > > >> @@ -452,10 +453,9 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin > >> fwrite(out.buf, 1, out.len, stdout); > >> putchar('\n'); > >> } > >> - strbuf_release(&err); > >> - strbuf_release(&out); > >> } > >> > >> + strbuf_release(&out); > > > > err is no longer released, and it is also not reset in the loop. > > That change is not mentioned in the commit message, but it should. > > Why is it safe? Probably because format_ref_array_item() only > > populates it if it also returns non-zero and then we end up dying > > anyway. > > > > That makes leak checking harder, though -- it's not easy to see if > > err hasn't simply been forgotten to be released. I'd just retain > > the strbuf_release() call at the end of the function -- it > > shouldn't have a measurable performance impact and documents that > > this function is cleaning up after itself. Thoughts? > > I should have responded to this comment before it was too late, > sorry. > > I am OK with documenting the assumption that we will die when err > gets populated without coming out of the loop and not releasing at > the end (because we would not have anything to release when we got > there). I am also OK with resetting(err) in the loop (which will > be a no-op as err.len would always be 0 at that point, or we would > have died) and releasing(err) at the end. I found it a bit funny > to be not resetting in the loop and releasing at the end, without > a comment that explains the thought behind it. > So a better solution is "without err buffer _reset() and _release()" and explain the reason for "not needing to be cleaned up" in the commit message? Thanks. -- ZheNing Hu