Skip to content

Commit 69c54c7

Browse files
gitstergitster
committed
Merge branch 'ma/leakplugs'
Memory leaks in various codepaths have been plugged. * ma/leakplugs: pack-bitmap[-write]: use `object_array_clear()`, don't leak object_array: add and use `object_array_pop()` object_array: use `object_array_clear()`, not `free()` leak_pending: use `object_array_clear()`, not `free()` commit: fix memory leak in `reduce_heads()` builtin/commit: fix memory leak in `prepare_index()`
2 parents 14a8168 + 4d01a7f commit 69c54c7

File tree

17 files changed

+75
-35
lines changed

17 files changed

+75
-35
lines changed

‎bisect.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,8 @@ static int check_ancestors(const char *prefix)
826826

827827
/* Clean up objects used, as they will be reused. */
828828
clear_commit_marks_for_object_array(&pending_copy, ALL_REV_FLAGS);
829-
free(pending_copy.objects);
829+
830+
object_array_clear(&pending_copy);
830831

831832
return res;
832833
}

‎builtin/checkout.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -797,18 +797,25 @@ static void orphaned_commit_warning(struct commit *old, struct commit *new)
797797
for_each_ref(add_pending_uninteresting_ref, &revs);
798798
add_pending_oid(&revs, "HEAD", &new->object.oid, UNINTERESTING);
799799

800+
/* Save pending objects, so they can be cleaned up later. */
800801
refs = revs.pending;
801802
revs.leak_pending = 1;
802803

804+
/*
805+
* prepare_revision_walk (together with .leak_pending = 1) makes us
806+
* the sole owner of the list of pending objects.
807+
*/
803808
if (prepare_revision_walk(&revs))
804809
die(_("internal error in revision walk"));
805810
if (!(old->object.flags & UNINTERESTING))
806811
suggest_reattach(old, &revs);
807812
else
808813
describe_detached_head(_("Previous HEAD position was"), old);
809814

815+
/* Clean up objects used, as they will be reused. */
810816
clear_commit_marks_for_object_array(&refs, ALL_REV_FLAGS);
811-
free(refs.objects);
817+
818+
object_array_clear(&refs);
812819
}
813820

814821
static int switch_branches(const struct checkout_opts *opts,

‎builtin/commit.c‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ static void refresh_cache_or_die(int refresh_flags)
335335
static const char *prepare_index(int argc, const char **argv, const char *prefix,
336336
const struct commit *current_head, int is_status)
337337
{
338-
struct string_list partial;
338+
struct string_list partial = STRING_LIST_INIT_DUP;
339339
struct pathspec pathspec;
340340
int refresh_flags = REFRESH_QUIET;
341341
const char *ret;
@@ -380,7 +380,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
380380
warning(_("Failed to update main cache tree"));
381381

382382
commit_style = COMMIT_NORMAL;
383-
return get_lock_file_path(&index_lock);
383+
ret = get_lock_file_path(&index_lock);
384+
goto out;
384385
}
385386

386387
/*
@@ -403,7 +404,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
403404
if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
404405
die(_("unable to write new_index file"));
405406
commit_style = COMMIT_NORMAL;
406-
return get_lock_file_path(&index_lock);
407+
ret = get_lock_file_path(&index_lock);
408+
goto out;
407409
}
408410

409411
/*
@@ -429,7 +431,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
429431
rollback_lock_file(&index_lock);
430432
}
431433
commit_style = COMMIT_AS_IS;
432-
return get_index_file();
434+
ret = get_index_file();
435+
goto out;
433436
}
434437

435438
/*
@@ -460,7 +463,6 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
460463
die(_("cannot do a partial commit during a cherry-pick."));
461464
}
462465

463-
string_list_init(&partial, 1);
464466
if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, &pathspec))
465467
exit(1);
466468

@@ -490,6 +492,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
490492
discard_cache();
491493
ret = get_lock_file_path(&false_lock);
492494
read_cache_from(ret);
495+
out:
496+
string_list_clear(&partial, 0);
497+
clear_pathspec(&pathspec);
493498
return ret;
494499
}
495500

‎builtin/fast-export.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -650,11 +650,10 @@ static void handle_tail(struct object_array *commits, struct rev_info *revs,
650650
{
651651
struct commit *commit;
652652
while (commits->nr) {
653-
commit = (struct commit *)commits->objects[commits->nr - 1].item;
653+
commit = (struct commit *)object_array_pop(commits);
654654
if (has_unshown_parent(commit))
655655
return;
656656
handle_commit(commit, revs, paths_of_changed_objects);
657-
commits->nr--;
658657
}
659658
}
660659

‎builtin/fsck.c‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,7 @@ static int traverse_reachable(void)
182182
if (show_progress)
183183
progress = start_delayed_progress(_("Checking connectivity"), 0);
184184
while (pending.nr) {
185-
struct object_array_entry *entry;
186-
struct object *obj;
187-
188-
entry = pending.objects + --pending.nr;
189-
obj = entry->item;
190-
result |= traverse_one_object(obj);
185+
result |= traverse_one_object(object_array_pop(&pending));
191186
display_progress(progress, ++nr);
192187
}
193188
stop_progress(&progress);

‎builtin/reflog.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ static int commit_is_complete(struct commit *commit)
126126
struct commit *c;
127127
struct commit_list *parent;
128128

129-
c = (struct commit *)study.objects[--study.nr].item;
129+
c = (struct commit *)object_array_pop(&study);
130130
if (!c->object.parsed && !parse_object(&c->object.oid))
131131
c->object.flags |= INCOMPLETE;
132132

@@ -182,8 +182,8 @@ static int commit_is_complete(struct commit *commit)
182182
found.objects[i].item->flags |= SEEN;
183183
}
184184
/* free object arrays */
185-
free(study.objects);
186-
free(found.objects);
185+
object_array_clear(&study);
186+
object_array_clear(&found);
187187
return !is_incomplete;
188188
}
189189

‎bundle.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,14 @@ int verify_bundle(struct bundle_header *header, int verbose)
157157
req_nr = revs.pending.nr;
158158
setup_revisions(2, argv, &revs, NULL);
159159

160+
/* Save pending objects, so they can be cleaned up later. */
160161
refs = revs.pending;
161162
revs.leak_pending = 1;
162163

164+
/*
165+
* prepare_revision_walk (together with .leak_pending = 1) makes us
166+
* the sole owner of the list of pending objects.
167+
*/
163168
if (prepare_revision_walk(&revs))
164169
die(_("revision walk setup failed"));
165170

@@ -176,8 +181,10 @@ int verify_bundle(struct bundle_header *header, int verbose)
176181
refs.objects[i].name);
177182
}
178183

184+
/* Clean up objects used, as they will be reused. */
179185
clear_commit_marks_for_object_array(&refs, ALL_REV_FLAGS);
180-
free(refs.objects);
186+
187+
object_array_clear(&refs);
181188

182189
if (verbose) {
183190
struct ref_list *r;

‎commit.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)
10861086
num_head = remove_redundant(array, num_head);
10871087
for (i = 0; i < num_head; i++)
10881088
tail = &commit_list_insert(array[i], tail)->next;
1089+
free(array);
10891090
return result;
10901091
}
10911092

‎diff-lib.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,6 @@ int index_differs_from(const char *def, int diff_flags,
549549
rev.diffopt.flags |= diff_flags;
550550
rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
551551
run_diff_index(&rev, 1);
552-
if (rev.pending.alloc)
553-
free(rev.pending.objects);
552+
object_array_clear(&rev.pending);
554553
return (DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0);
555554
}

‎object.c‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,19 @@ static void object_array_release_entry(struct object_array_entry *ent)
353353
free(ent->path);
354354
}
355355

356+
struct object *object_array_pop(struct object_array *array)
357+
{
358+
struct object *ret;
359+
360+
if (!array->nr)
361+
return NULL;
362+
363+
ret = array->objects[array->nr - 1].item;
364+
object_array_release_entry(&array->objects[array->nr - 1]);
365+
array->nr--;
366+
return ret;
367+
}
368+
356369
void object_array_filter(struct object_array *array,
357370
object_array_each_func_t want, void *cb_data)
358371
{

0 commit comments

Comments
 (0)