Re: [RFC] http clone does not checkout working tree

On Wed, 4 Jun 2008, Jeff King wrote:

Ack. This is correct.

That said, a *lot* of code does this pattern (freeing the tree buffer after use, without marking the tree non-parsed), and I suspect the only reason I'm blamed is because this got copied from some other use of that same model.

Normally it's fine, because the whole object use is temporary, but as you point out, doing things in the same process will re-use old object info. It's one of the subtler implications of doing built-ins without fork/exec.

It is possible that we should clean this up by adding some kind of

	static void forget_tree(struct tree *tree)
	{
		free(tree->buffer);
		tree->buffer = NULL;
		tree->size = 0;
		tree->parsed = 0;
	}

to make this more robust and obvious. That said, a lot of the users are basically the type

	if (parse_tree(tree) < 0)
		die(..);
	init_tree_desc(&desc, tree->buffer, tree->size);
	while (tree_entry(&desc, &entry)) {
		...
	}
	forget_tree();

and quite frankly, it's rather possible that we should get rid of the "void *buffer" and "unsigned long size" in the tree *entirely*, because the above would likely be better written as

	buffer = read_tree_desc(&desc);
	while (tree_entry(&desc, &entry)) {
		...
	}
	free(buffer);

and make "struct tree" smaller, and not ever need parsing at all!

I think that realisitcially, all tree users are of that format, and nobody really wants to save the buffer (because buffer re-use is fairly unlikely, an re-generating it isn't all that expensive).

But that would be a much bigger patch. And maybe I'm wrong, and some uses really do want the longer-term caching because they end up re-using the tree a lot. So it would need more thinking about.

		Linus