Re: What's cooking in git.git (Oct 2013, #03; Wed, 16)

From

Karsten Blees <karsten.blees@gmail.com>

Date

Oct 18, 2013, 00:42 UTC

Message-ID

<526083E7.6080701@gmail.com>

In-Reply-To

<xmqqmwm7y5bf.fsf@gitster.dls.corp.google.com>

Am 17.10.2013 23:07, schrieb Junio C Hamano:

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Karsten Blees <karsten.blees@gmail.com> writes:
>>
>>> Am 16.10.2013 23:43, schrieb Junio C Hamano:
>>>> * kb/fast-hashmap (2013-09-25) 6 commits
>>>>  - fixup! diffcore-rename.c: simplify finding exact renames
>>>>  - diffcore-rename.c: use new hash map implementation
>>>>  - diffcore-rename.c: simplify finding exact renames
>>>>  - diffcore-rename.c: move code around to prepare for the next patch
>>>>  - buitin/describe.c: use new hash map implementation
>>>>  - add a hashtable implementation that supports O(1) removal
>>>>
>>>
>>> I posted a much more complete v3 [1], but somehow missed Jonathan's fixup! commit.
>>
>> Thanks; I'll replace the above with v3 and squash the fix-up in.
> 
> Interestingly, v3 applied on 'maint' and then merged to 'master'
> seems to break t3600 and t7001 with a coredump.
> 
> It would conflict with es/name-hash-no-trailing-slash-in-dirs that
> has been cooking in 'next', too; the resolution might be trivial but
> I didn't look too deeply into it.
> 

The coredumps are caused by my patch #10, which free()s cache_entries when they are removed, in combination with submodule.c::stage_updated_gitmodules (5fee9952 "submodule.c: add .gitmodules staging helper functions"), which removes a cache_entry, then modifies and re-adds the (now) free()d memory.

Can't we just use add_file_to_cache here (which replaces cache_entries by creating a copy)?

diff --git a/submodule.c b/submodule.c
index 1905d75..e388487 100644
--- a/submodule.c
+++ b/submodule.c
@@ -116,30 +116,7 @@ int remove_path_from_gitmodules(const char *path)
 
 void stage_updated_gitmodules(void)
 {
-       struct strbuf buf = STRBUF_INIT;
-       struct stat st;
-       int pos;
-       struct cache_entry *ce;
-       int namelen = strlen(".gitmodules");
-
-       pos = cache_name_pos(".gitmodules", namelen);
-       if (pos < 0) {
-               warning(_("could not find .gitmodules in index"));
-               return;
-       }
-       ce = active_cache[pos];
-       ce->ce_flags = namelen;
-       if (strbuf_read_file(&buf, ".gitmodules", 0) < 0)
-               die(_("reading updated .gitmodules failed"));
-       if (lstat(".gitmodules", &st) < 0)
-               die_errno(_("unable to stat updated .gitmodules"));
-       fill_stat_cache_info(ce, &st);
-       ce->ce_mode = ce_mode_from_stat(ce, st.st_mode);
-       if (remove_cache_entry_at(pos) < 0)
-               die(_("unable to remove .gitmodules from index"));
-       if (write_sha1_file(buf.buf, buf.len, blob_type, ce->sha1))
-               die(_("adding updated .gitmodules failed"));
-       if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
+       if (add_file_to_cache(".gitmodules", 0))
                die(_("staging updated .gitmodules failed"));
 }