From: Martin Koegler <mkoegler@auto.tuwien.ac.at>
Date: Thu, 11 Dec 2008 06:27:53 GMT
Subject: Re: git fsck segmentation fault
Message-ID: <20081211062753.GA17683@auto.tuwien.ac.at>
In-Reply-To: <7vljunwidr.fsf@gitster.siamese.dyndns.org>

On Wed, Dec 10, 2008 at 06:33:20PM -0800, Junio C Hamano wrote:
> mkoegler@auto.tuwien.ac.at (Martin Koegler) writes:
> A similar change would be needed for other callers of fsck_walk(), no?
> There seem to be one in builtin-unpack-objects.c (check_object calls
> fsck_walk as itself as the callback). 

buitin-unpack-objects.c is different. First, its intended for the
small case [default unpack_limit is 100; it keeps the unpacked content
of trees/commits in memory], which will not overflow the
stack. Second, it may only write an object after all of its connected
objects have been written out. So it would need a totally different
logic.

> Another caller is in index-pack.c (sha1_object() calls fsck_walk with
> mark_link as the callback), but I do not think it would  recurse for the
> depth of the history, so we are safe there.

mark_link only sets a flag on the direct connected objects, so yes, it
needs no change.

> I initially expected that the fix would be to introduce this "userspace
> work queue" (i.e. your objectstack) to be maintained on the
> fsck.c:fsck_walk() side (perhaps as an extra parameter to an actual queue
> for reentrancy), not by making the callee not to recurse, though.

fsck_walk has been designed to call a function on all directly
connected objected. There are callers, which expected this behaviour
(eg. index-pack, mark_used in fsck).

mfg Martin Kögler