Hi Junio, On Sun, Apr 05, 2020 at 02:33:08PM -0700, Junio C Hamano wrote: > Denton Liu <liu.denton@xxxxxxxxx> writes: > > > In read_populate_opts(), we call read_oneliner() _after_ calling > > strbuf_release(). This means that `buf` is leaked at the end of the > > function. > > I do not think the above makes much sense. _release() will free the > piece of memory occupied by .buf and reinitializes the strbuf, and > in doing so there is no leak. read_oneliner() called after it > allocates and reads into there. Freeing the resource needs to be > done after the caller is done with what read_oneliner() has read. > > There is a leak, because read_oneliner() gets called and before the > code has a chance to do strbuf_reease() there is an error return. > That does not have anything to do with the call to strbuf_release() > in the middle of the function. There is also a leak in the case where we don't take the early return since, before this patch, the read_oneliner() has no strbuf_release() following it which means buf gets leaked. > But that leak has nothing to do with the release called before > read_oneliner(). What I meant to say in my paragraph is essentially, "strbuf_release() is placed too early, there is a read_oneliner() after that nullifies the effects of strbuf_release()". I can rewrite this to be more clear. Thanks, Denton
