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

[PATCH v2 16/17] fetch_pack(): free matching heads

From
mhagger@alum.mit.edu <mhagger@alum.mit.edu>
Date
Aug 25, 2012, 06:44 UTC
Message-ID
<1345877067-11841-17-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1345877067-11841-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>

fetch_pack() used to delete entries from the input list (*nr_heads, heads) and drop them on the floor. (Even earlier versions dropped some names on the floor and modified others.) This forced fetch_refs_via_pack() to create a separate copy of the original list so that it could free all of the names.

Instead, teach fetch_pack() to free any names that it discards from the list, and change fetch_refs_via_pack() to free only the remaining (unmatched) names.

Document the change in the function comment in the header file.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/fetch-pack.c | 4 +++-
 fetch-pack.h         | 7 ++++---
 transport.c          | 9 +++------
 3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index c49dadf..1bc4599 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -554,7 +554,7 @@ static void filter_refs(struct ref **refs, int *nr_heads, char **heads)
 					break;
 				else if (cmp == 0) { /* definitely have it */
 					return_refs[matched++] = ref;
-					head_pos++;
+					free(heads[head_pos++]);
 					break;
 				}
 				else { /* might have it; keep looking */
@@ -844,6 +844,8 @@ static int remove_duplicates(int nr_heads, char **heads)
 	for (src = dst = 1; src < nr_heads; src++)
 		if (strcmp(heads[src], heads[dst-1]))
 			heads[dst++] = heads[src];
+		else
+			free(heads[src]);
 	return dst;
 }
 
diff --git a/fetch-pack.h b/fetch-pack.h
index 915858e..d1860eb 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -19,9 +19,10 @@ struct fetch_pack_args {
 
 /*
  * (*nr_heads, heads) is an array of pointers to the full names of
- * remote references that should be updated from.  On return, both
- * will have been changed to list only the names that were not found
- * on the remote.
+ * remote references that should be updated from.  The names must have
+ * been allocated from the heap.  The names that are found in the
+ * remote will be removed from the list and freed; the others will be
+ * left in the front of the array with *nr_heads adjusted accordingly.
  */
 struct ref *fetch_pack(struct fetch_pack_args *args,
 		       int fd[], struct child_process *conn,
diff --git a/transport.c b/transport.c
index 3c38d44..f94b753 100644
--- a/transport.c
+++ b/transport.c
@@ -519,8 +519,6 @@ static int fetch_refs_via_pack(struct transport *transport,
 {
 	struct git_transport_data *data = transport->data;
 	char **heads = xmalloc(nr_heads * sizeof(*heads));
-	char **origh = xmalloc(nr_heads * sizeof(*origh));
-	int orig_nr_heads = nr_heads;
 	const struct ref *refs;
 	char *dest = xstrdup(transport->url);
 	struct fetch_pack_args args;
@@ -539,7 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 	args.depth = data->options.depth;
 
 	for (i = 0; i < nr_heads; i++)
-		origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
+		heads[i] = xstrdup(to_fetch[i]->name);
 
 	if (!data->got_remote_heads) {
 		connect_setup(transport, 0, 0);
@@ -559,9 +557,8 @@ static int fetch_refs_via_pack(struct transport *transport,
 
 	free_refs(refs_tmp);
 
-	for (i = 0; i < orig_nr_heads; i++)
-		free(origh[i]);
-	free(origh);
+	for (i = 0; i < nr_heads; i++)
+		free(heads[i]);
 	free(heads);
 	free(dest);
 	return (refs ? 0 : -1);

-- 
1.7.11.3
Previous: mhagger@alum.mit.eduNext: mhagger@alum.mit.edu
Message 17 of 19 in “Clean up how fetch_pack() handles the heads list”
  1. 00/17 Clean up how fetch_pack() handles the heads listmhagger@alum.mit.edu, Aug 25, 2012
  2. 01/17 t5500: add tests of error output for missing refsmhagger@alum.mit.edu, Aug 25, 2012
  3. 02/17 Rename static function fetch_pack() to http_fetch_pack()mhagger@alum.mit.edu, Aug 25, 2012
  4. 03/17 Fix formatting.mhagger@alum.mit.edu, Aug 25, 2012
  5. 04/17 Name local variables more consistentlymhagger@alum.mit.edu, Aug 25, 2012
  6. 05/17 Do not check the same head_pos twicemhagger@alum.mit.edu, Aug 25, 2012
  7. 06/17 Let fetch_pack() inform caller about number of unique headsmhagger@alum.mit.edu, Aug 25, 2012
  8. 07/17 Pass nr_heads to do_pack_ref() by referencemhagger@alum.mit.edu, Aug 25, 2012
  9. 08/17 Pass nr_heads to everything_local() by referencemhagger@alum.mit.edu, Aug 25, 2012
  10. 09/17 Pass nr_heads to filter_refs() by referencemhagger@alum.mit.edu, Aug 25, 2012
  11. 10/17 Remove ineffective optimizationmhagger@alum.mit.edu, Aug 25, 2012
  12. 11/17 filter_refs(): do not leave gaps in return_refsmhagger@alum.mit.edu, Aug 25, 2012
  13. 12/17 filter_refs(): compress unmatched refs in heads arraymhagger@alum.mit.edu, Aug 25, 2012
  14. 13/17 cmd_fetch_pack: return early if finish_connect() returns an errormhagger@alum.mit.edu, Aug 25, 2012
  15. 14/17 Report missing refs even if no existing refs were receivedmhagger@alum.mit.edu, Aug 25, 2012
  16. 15/17 cmd_fetch_pack(): simplify computation of return valuemhagger@alum.mit.edu, Aug 25, 2012
  17. 16/17 fetch_pack(): free matching headsmhagger@alum.mit.edu, Aug 25, 2012
  18. 17/17 filter_refs(): simplify logicmhagger@alum.mit.edu, Aug 25, 2012
  19. Junio C HamanoAug 26, 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.