Re: [PATCH 1/3] Add read_cache_from() and discard_cache()

Hi,

On Fri, 30 Jun 2006, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

+int discard_cache()
+{
+	int ret;
+	
+	if (cache_mmap == NULL)
+		return 0;
+	ret = munmap(cache_mmap, cache_mmap_size);
+	cache_mmap = NULL;
+	cache_mmap_size = 0;
+	active_nr = active_cache_changed = 0;
+	/* no need to throw away allocated active_cache */
+	return ret;
+}
+

I haven't been following the details of the patches in this
thread while they are being cooked actively, but two things to
look out for are:

 - I am guessing you run discard_cache() because you want to
   read in a new cache (or start from a clean slate).  I am not
   sure what you are doing with the old cache tree data
   structure.  If you are starting from a clean slate
   (i.e. there is no read_cache_from() after you call
   discard_cache), you would probably need to discard the old
   cache tree; otherwise your next write-tree may produce an
   incorrect index file.  If you keep the old one and later
   swap it in, the problem might be even more severe.

True, I missed that one. But it is just a call to 
cache_tree_free(active_cache_tree); in discard_cache(), right?

 - index_timestamp is left as the old value in this patch when
   you switch cache using read_cache_from() directly.  I have a
   suspicion you may be bitten by "Racy Git" problem, especially
   because the operations are supposed to happen quickly thanks
   to the effort of you two ;-) increasing the risks that the
   file timestamp of the working tree file and the cached entry
   match.

Yes. Again, just one line to discard_cache(), right?

	index_file_timestamp = 0;

If there is more to it, please don't let me die dumb.

Ciao,
Dscho