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

[PATCH 4/4] bundle: keep around names passed to add_pending_object()

From
Thomas Rast <trast@student.ethz.ch>
Date
Mar 1, 2012, 21:40 UTC
Message-ID
<6fba13f255f76481dc9098b5e8fbb33bd10735cb.1330637923.git.trast@student.ethz.ch>
In-Reply-To
<cover.1330637923.git.trast@student.ethz.ch>

The 'name' field passed to add_pending_object() is used to later deduplicate in object_array_remove_duplicates().

git-bundle had a bug in this area since 18449ab (git-bundle: avoid packing objects which are in the prerequisites, 2007-03-08): it passed the name of each boundary object in a static buffer. In other words, all that object_array_remove_duplicates() saw was the name of the *last* added boundary object.

The recent switch to a strbuf in bc2fed4 (bundle: use a strbuf to scan the log for boundary commits, 2012-02-22) made this slightly worse: we now free the buffer at the end, so it is not even guaranteed that it still points into addressable memory by the time object_array_remove_ duplicates looks at it. On the plus side however, it was now detectable by valgrind.

The fix is easy: pass a copy of the string to add_pending_object.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 bundle.c         |    2 +-
 t/t5510-fetch.sh |   16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/bundle.c b/bundle.c
index 7a760db..d9cfd90 100644
--- a/bundle.c
+++ b/bundle.c
@@ -273,7 +273,7 @@ int create_bundle(struct bundle_header *header, const char *path,
 			if (!get_sha1_hex(buf.buf + 1, sha1)) {
 				struct object *object = parse_object(sha1);
 				object->flags |= UNINTERESTING;
-				add_pending_object(&revs, object, buf.buf);
+				add_pending_object(&revs, object, xstrdup(buf.buf));
 			}
 		} else if (!get_sha1_hex(buf.buf, sha1)) {
 			struct object *object = parse_object(sha1);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index dd035bf..8b914f9 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -440,4 +440,20 @@ test_expect_success "should be able to fetch with duplicate refspecs" '
 	git fetch three
 '
 
+
+test_expect_success 'all boundary commits are excluded' '
+	test_commit base &&
+	test_commit oneside &&
+	git checkout HEAD^ &&
+	test_commit otherside &&
+	git checkout master &&
+	test_tick &&
+	git merge otherside &&
+	ad=$(git log --no-walk --format=%ad HEAD) &&
+	git bundle create twoside-boundary.bdl master --since="$ad" &&
+	convert_bundle_to_pack <twoside-boundary.bdl >twoside-boundary.pack &&
+	pack=$(git index-pack --fix-thin --stdin <twoside-boundary.pack) &&
+	test_bundle_object_count .git/objects/pack/pack-${pack##pack	}.pack 3
+'
+
 test_done
-- 
1.7.9.2.467.g7fee4
Previous: Junio C HamanoNext: Junio C Hamano
Message 9 of 11 in “Another bundle fix: reading freed memory”
  1. 0/4 Another bundle fix: reading freed memoryThomas Rast, Mar 1, 2012
  2. 1/4 t5510: refactor bundle->pack conversionThomas Rast, Mar 1, 2012
  3. 2/4 t5510: fix indent with spacesThomas Rast, Mar 1, 2012
  4. 3/4 t5510: ensure we stay in the toplevel test dirThomas Rast, Mar 1, 2012
  5. Junio C HamanoMar 1, 2012
  6. Thomas RastMar 1, 2012
  7. Junio C HamanoMar 1, 2012
  8. Junio C HamanoMar 1, 2012
  9. 4/4 bundle: keep around names passed to add_pending_object()Thomas Rast, Mar 1, 2012
  10. Junio C HamanoMar 1, 2012
  11. Thomas RastMar 1, 2012

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.