Ok.. so there's been a couple attempts to patch the leak that were all
wrong due to mixed memory management for that array. Here's a seed for
discussion on how to plug that leak. Some would argue that it's not
leaking enough to fix, but for those that want to turn git into a library,
the lifetime of the cache could end up not being short any more, so it's
worth discussing how to fix it.
The q&d fix in this patch isn't elegant, but gets the job done. More
interesting could be to have the entry itself contain a state bit, though
that wastes storage space.
Two basic changes:
1) introduce a set_active_cache() api and change all 'active_cache[i] = ce'
calls to use it.
2) add a active_cache_malloced array to parallel the active_cache array.
I don't like #2, but see that q&d comment. :)
It's only lightly tested as I'm still trying to wrap my head around how to
actually use git and git-pasky.
I was tempted to add a get_cache_entry api as well, so that nothing
outside of read-cache.c touched active_cache directly, but that can come
next.
Later,
Brad
--- cache.h
+++ cache.h 2005-04-16 23:08:37.000000000 -0700
@@ -88,6 +88,7 @@
extern int read_cache(void);
extern int write_cache(int newfd, struct cache_entry **cache, int entries);
extern int cache_name_pos(const char *name, int namelen);
+extern int set_cache_entry(struct cache_entry *ce, int pos, int
malloced_entry);
extern int add_cache_entry(struct cache_entry *ce, int ok_to_add);
extern int remove_file_from_cache(char *path);
extern int cache_match_stat(struct cache_entry *ce, struct stat *st);
--- read-cache.c
+++ read-cache.c 2005-04-16 23:32:34.000000000 -0700
@@ -8,6 +8,7 @@
const char *sha1_file_directory = NULL;
struct cache_entry **active_cache = NULL;
+static int * active_cache_malloced = NULL;
unsigned int active_nr = 0, active_alloc = 0;
void usage(const char *err)
@@ -381,6 +382,15 @@
return ce_namelen(b) == len && !memcmp(a->name, b->name, len);
}
+int set_cache_entry(struct cache_entry *ce, int pos, int malloced_entry)
+{
+ if (active_cache_malloced[pos])
+ free(active_cache[pos]);
+ active_cache[pos] = ce;
+ active_cache_malloced[pos] = malloced_entry;
+ return 0;
+}
+
int add_cache_entry(struct cache_entry *ce, int ok_to_add)
{
int pos;
@@ -389,7 +399,7 @@
/* existing match? Just replace it */
if (pos >= 0) {
- active_cache[pos] = ce;
+ set_cache_entry(ce, pos, 0);
return 0;
}
pos = -pos-1;
@@ -414,13 +424,16 @@
if (active_nr == active_alloc) {
active_alloc = alloc_nr(active_alloc);
active_cache = realloc(active_cache, active_alloc *
sizeof(struct cache_entry *));
+ active_cache_malloced = realloc(active_cache, active_alloc *
sizeof(int));
}
/* Add it in.. */
active_nr++;
- if (active_nr > pos)
+ if (active_nr > pos) {
memmove(active_cache + pos + 1, active_cache + pos, (active_nr
- pos - 1) * sizeof(ce));
- active_cache[pos] = ce;
+ memmove(active_cache_malloced + pos + 1, active_cache_malloced
+ pos, (active_nr - pos - 1) * sizeof(int));
+ }
+ set_cache_entry(ce, pos, 1);
return 0;
}
@@ -482,12 +495,13 @@
active_nr = ntohl(hdr->hdr_entries);
active_alloc = alloc_nr(active_nr);
active_cache = calloc(active_alloc, sizeof(struct cache_entry *));
+ active_cache_malloced = calloc(active_alloc, sizeof(int));
offset = sizeof(*hdr);
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = map + offset;
offset = offset + ce_size(ce);
- active_cache[i] = ce;
+ set_cache_entry(ce, i, 0);
}
return active_nr;
--- update-cache.c
+++ update-cache.c 2005-04-16 23:33:28.000000000 -0700
@@ -199,11 +199,14 @@
struct cache_entry *ce = active_cache[i];
struct cache_entry *new = refresh_entry(ce);
+ if (new == ce)
+ continue;
+
if (!new) {
printf("%s: needs update\n", ce->name);
continue;
}
- active_cache[i] = new;
+ set_cache_entry(new, i, 1);
}
}
