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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux