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); } }