Re: [RFH] index_name_exists() conversion

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Feb 21, 2008, 16:15 UTC
Message-ID: <alpine.LFD.1.00.0802210806540.7833@woody.linux-foundation.org>
In-Reply-To: <7vy79evfsn.fsf@gitster.siamese.dyndns.org>

On Thu, 21 Feb 2008, Junio C Hamano wrote:

Show 12 quoted lines

>
>          struct dir_entry *dir_add_name(struct dir_struct *dir, const c...
>          {
>         -	if (cache_name_pos(pathname, len) >= 0)
>         +	if (cache_name_exists(pathname, len))
>                         return NULL;
> 
>                 ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
> 
> This used to check if there is a stage#0 entry of that name in
> the index.  But the name-hash based index look-up does not care
> about the stage as far as I can tell.

I think the new code is much more correct, and we should always care about whether the name exists in the index, not whether it exists in stage#0.

IOW, if we ask for "show other files", we definitely want to ignore paths we already know about, and conversely, if we do an operation that is supposed to care about files in a directory we already know about, then an unmerged entry should count the same as a merged one.

> This could be related to the recent "status segfaults, bisected
> to cf558704" issue posted to the list.  I dunno.

Is there a backtrace for that? It's certainly possible that some strange path depended on only matching merged entries, but I would consider that unlikely and buggy.

The segfault is more likely related to something more subtle, like actually freeing a "cache_entry" after having removed it from the index (which we should never do - they were always supposed to stay around because they might not have been allocated with 'malloc()' in the first place, but with the name indexing thing it now is *always* a bug to do it, even if you _did_ allocate it with free()).

(Also, some code used to just reuse the same cache entry multiple times, and that got illegal for the same reason).

		Linus