On Fri, Sep 20, 2019 at 11:21:53AM -0400, Derrick Stolee wrote:
> On 9/19/2019 5:47 PM, SZEDER Gábor wrote:
> > The 'if (deref) { ... }' condition near the beginning of the recursive
> > name_rev() function can only ever be true in the first invocation,
> > because the 'deref' parameter is always 0 in the subsequent recursive
> > invocations.
> >
> > Extract this condition from the recursion into name_rev()'s caller and
> > drop the function's 'deref' parameter. This makes eliminating the
> > recursion a bit easier to follow, and it will be moved back into
> > name_rev() after the recursion is elminated.
s/elminated/eliminated/
> > Furthermore, drop the condition that die()s when both 'deref' and
> > 'generation' are non-null (which should have been a BUG() to begin
> > with).
>
> These changes seem sensible. I look forward to seeing how deref is
> reintroduced.
>
> > Note that this change reintroduces the memory leak that was plugged in
> > in commit 5308224633 (name-rev: avoid leaking memory in the `deref`
> > case, 2017-05-04), but a later patch in this series will plug it in
> > again.
>
> The memory leak is now for "tip_name" correct? Just tracking to make
> sure it gets plugged later.
Yes, it's 'tip_name' (the one returned by xstrfmt()).
> > - if (deref) {
> > - tip_name = to_free = xstrfmt("%s^0", tip_name);
> > + if (deref)
> > + tip_name = xstrfmt("%s^0", path);
> > + else
> > + tip_name = xstrdup(path);
