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

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@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 the 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 mhag...@alum.mit.edu
---

This change forces callers to allocate names on the heap.  But the two
existing callers did so already, and the function already modified the
list, so I think the new style is no more intrusive than the old.

 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 6c1a48e..9650d04 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;
-   match_pos++;
+   free(heads[match_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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 10:10:41AM +0200, mhag...@alum.mit.edu wrote:

 From: Michael Haggerty mhag...@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 the all of the names.

Minor typo: s/the all/all/.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html