From bfffb48c5d520a5f181716b0330774a03cd3fa39 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 5 Sep 2017 08:15:21 -0400 Subject: stop leaking lock structs in some simple cases Now that it's safe to declare a "struct lock_file" on the stack, we can do so (and avoid an intentional leak). These leaks were found by running t0000 and t0001 under valgrind (though certainly other similar leaks exist and just don't happen to be exercised by those tests). Initializing the lock_file's inner tempfile with NULL is not strictly necessary in these cases, but it's a good practice to model. It means that if we were to call a function like rollback_lock_file() on a lock that was never taken in the first place, it becomes a quiet noop (rather than undefined behavior). Likewise, it's always safe to rollback_lock_file() on a file that has already been committed or deleted, since that operation is a noop on an inactive lockfile (and that's why the case in config.c can drop the "if (lock)" check as we move away from using a pointer). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache-tree.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) (limited to 'cache-tree.c') diff --git a/cache-tree.c b/cache-tree.c index 2690113a6a..71d092ed51 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -603,16 +603,10 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, const char *index_path, int flags, const char *prefix) { int entries, was_valid, newfd; - struct lock_file *lock_file; + struct lock_file lock_file = LOCK_INIT; int ret = 0; - /* - * We can't free this memory, it becomes part of a linked list - * parsed atexit() - */ - lock_file = xcalloc(1, sizeof(struct lock_file)); - - newfd = hold_lock_file_for_update(lock_file, index_path, LOCK_DIE_ON_ERROR); + newfd = hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR); entries = read_index_from(index_state, index_path); if (entries < 0) { @@ -632,7 +626,7 @@ int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, co goto out; } if (0 <= newfd) { - if (!write_locked_index(index_state, lock_file, COMMIT_LOCK)) + if (!write_locked_index(index_state, &lock_file, COMMIT_LOCK)) newfd = -1; } /* Not being able to write is fine -- we are only interested @@ -657,7 +651,7 @@ int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, co out: if (0 <= newfd) - rollback_lock_file(lock_file); + rollback_lock_file(&lock_file); return ret; } -- cgit v1.2.1