@matheusbsbs / matheus-bagliano / commits / dd1055ed59

builtin/commit: fix memory leak in `prepare_index()`

Release `pathspec` and the string list `partial`. When we clear the string list, make sure we do not free the `util` pointers. That would result in double-freeing, since we set them up as `item->util = item` in `list_paths()`. Initialize the string list early, so that we can always release it. That introduces some unnecessary overhead in various code paths, but means there is one and only one way out of the function. If we ever accumulate more things we need to free, it should be straightforward to do so. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed Sep 23, 2017 at 01:34 UTC dd1055ed594f8fef18779cce3cd921c4ac66cf9c

1 file changed +10 -5

builtin/commit.c

+10 -5

@@ -336,7 +336,7 @@ static void refresh_cache_or_die(int refresh_flags)

336 static const char *prepare_index(int argc, const char **argv, const char *prefix,

337 const struct commit *current_head, int is_status)

338 {

339 - struct string_list partial;

339 + struct string_list partial = STRING_LIST_INIT_DUP;

340 struct pathspec pathspec;

341 int refresh_flags = REFRESH_QUIET;

342 const char *ret;

@@ -381,7 +381,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

381 warning(_("Failed to update main cache tree"));

382

383 commit_style = COMMIT_NORMAL;

384 - return get_lock_file_path(&index_lock);

384 + ret = get_lock_file_path(&index_lock);

385 + goto out;

386 }

387

388 /*

@@ -404,7 +405,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

405 if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))

406 die(_("unable to write new_index file"));

407 commit_style = COMMIT_NORMAL;

407 - return get_lock_file_path(&index_lock);

408 + ret = get_lock_file_path(&index_lock);

409 + goto out;

410 }

411

412 /*

@@ -430,7 +432,8 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

432 rollback_lock_file(&index_lock);

433 }

434 commit_style = COMMIT_AS_IS;

433 - return get_index_file();

435 + ret = get_index_file();

436 + goto out;

437 }

438

439 /*

@@ -461,7 +464,6 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

464 die(_("cannot do a partial commit during a cherry-pick."));

465 }

466

464 - string_list_init(&partial, 1);

467 if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, &pathspec))

468 exit(1);

469

@@ -491,6 +493,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

493 discard_cache();

494 ret = get_lock_file_path(&false_lock);

495 read_cache_from(ret);

496 +out:

497 + string_list_clear(&partial, 0);

498 + clear_pathspec(&pathspec);

499 return ret;

500 }

501