git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 01/14] submodule: don't access the .gitmodules cache entry after removing it

From
Karsten Blees <karsten.blees@gmail.com>
Date
Nov 7, 2013, 14:33 UTC
Message-ID
<527BA4C7.4020700@gmail.com>
In-Reply-To
<527BA483.6040803@gmail.com>

Commit 5fee995244e introduced the stage_updated_gitmodules() function to add submodule configuration updates to the index. It assumed that even after calling remove_cache_entry_at() the same cache entry would still be valid. This was true in the old days, as cache entries could never be freed, but that is not so sure in the present as there is ongoing work to free removed cache entries, which makes this code segfault.

Fix that by calling add_file_to_cache() instead of open coding it. Also remove the "could not find .gitmodules in index" warning, as that won't happen in regular use cases (and by then just silently adding it to the index we do the right thing).

Thanks-to: Karsten Blees <karsten.blees@gmail.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 submodule.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)
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"));
 }
 
-- 
1.8.4.msysgit.0.12.g88f5ed0
Previous: Karsten BleesNext: Heiko Voigt
Message 2 of 26 in “New hash table implementation”
  1. 00/14 New hash table implementationKarsten Blees, Nov 7, 2013
  2. 01/14 submodule: don't access the .gitmodules cache entry after removing itKarsten Blees, Nov 7, 2013
  3. Heiko VoigtNov 7, 2013
  4. 02/14 add a hashtable implementation that supports O(1) removalKarsten Blees, Nov 7, 2013
  5. Junio C HamanoNov 7, 2013
  6. Karsten BleesNov 8, 2013
  7. Philip OakleyNov 8, 2013
  8. Junio C HamanoNov 8, 2013
  9. Karsten BleesNov 13, 2013
  10. 03/14 buitin/describe.c: use new hash map implementationKarsten Blees, Nov 7, 2013
  11. 04/14 diffcore-rename.c: move code around to prepare for the next patchKarsten Blees, Nov 7, 2013
  12. 05/14 diffcore-rename.c: simplify finding exact renamesKarsten Blees, Nov 7, 2013
  13. 06/14 diffcore-rename.c: use new hash map implementationKarsten Blees, Nov 7, 2013
  14. 07/14 name-hash.c: use new hash map implementation for directoriesKarsten Blees, Nov 7, 2013
  15. 08/14 name-hash.c: remove unreferenced directory entriesKarsten Blees, Nov 7, 2013
  16. 09/14 name-hash.c: use new hash map implementation for cache entriesKarsten Blees, Nov 7, 2013
  17. 10/14 name-hash.c: remove cache entries instead of marking them CE_UNHASHEDKarsten Blees, Nov 7, 2013
  18. 11/14 remove old hash.[ch] implementationKarsten Blees, Nov 7, 2013
  19. 12/14 fix 'git update-index --verbose --again' outputKarsten Blees, Nov 7, 2013
  20. Junio C HamanoNov 7, 2013
  21. Karsten BleesNov 8, 2013
  22. 13/14 builtin/update-index.c: cleanup update_oneKarsten Blees, Nov 7, 2013
  23. Junio C HamanoNov 7, 2013
  24. Karsten BleesNov 8, 2013
  25. 14/14 read-cache.c: fix memory leaks caused by removed cache entriesKarsten Blees, Nov 7, 2013
  26. Junio C HamanoNov 7, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.