Re: [PATCH] fast-import: avoid running end_packfile recursively

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> When an import has finished, we run end_packfile() to
> finalize the data and move the packfile into place. If this
> process fails, we call die() and end up in our die_nicely()
> handler.  Which unfortunately includes running end_packfile
> to save any progress we made. We enter the function again,
> and start operating on the pack_data struct while it is in
> an inconsistent state, leading to a segfault.
> ... This new problem is
> quite similar, except that we are worried about calling
> die() _during_ end_packfile, not right after. Ideally we
> would simply set pack_data to NULL as soon as we enter the
> function, and operate on a copy of the pointer.

Nicely analyzed and well done.

> Unfortunately, it is not so easy. pack_data is a global, and
> end_packfile calls into other functions which operate on the
> global directly. We would have to teach each of these to
> take an argument, and there is no guarantee that we would
> catch all of the spots.

Well, you can rename the global to something else to make sure ;-)
But I think that the approach with a simple flag is better.

If we were planning to do the global-to-parameter surgery for other
reasons (perhaps need to make things reentrant?) then the equation
might become different, but I do not think we are doing that right
now, so...

Thanks.