Re: [PATCH] [GSOC] ref-filter: get rid of show_ref_array_item

Junio C Hamano [off-list ref] 于2021年4月21日周三 上午2:00写道：

René Scharfe. [off-list ref] writes:

quoted

quoted

@@ -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