From f132a127eebb8f1b14b87fc317ca68278fceb2e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Thu, 5 Oct 2017 22:32:03 +0200 Subject: sha1_file: do not leak `lock_file` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is no longer any need to allocate and leak a `struct lock_file`. Initialize it on the stack instead. Before this patch, we set `lock = NULL` to signal that we have already rolled back, and that we should not do any more work. We need to take another approach now that we cannot assign NULL. We could, e.g., use `is_lock_file_locked()`. But we already have another variable that we could use instead, `found`. Its scope is only too small. Bump `found` to the scope of the whole function and rearrange the "roll back or write?"-checks to a straightforward if-else on `found`. This also future-proves the code by making it obvious that we intend to take exactly one of these paths. Improved-by: Jeff King Signed-off-by: Martin Ă…gren Signed-off-by: Junio C Hamano diff --git a/sha1_file.c b/sha1_file.c index 5a20148..1d17470 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -456,19 +456,19 @@ struct alternate_object_database *alloc_alt_odb(const char *dir) void add_to_alternates_file(const char *reference) { - struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); + struct lock_file lock = LOCK_INIT; char *alts = git_pathdup("objects/info/alternates"); FILE *in, *out; + int found = 0; - hold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR); - out = fdopen_lock_file(lock, "w"); + hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR); + out = fdopen_lock_file(&lock, "w"); if (!out) die_errno("unable to fdopen alternates lockfile"); in = fopen(alts, "r"); if (in) { struct strbuf line = STRBUF_INIT; - int found = 0; while (strbuf_getline(&line, in) != EOF) { if (!strcmp(reference, line.buf)) { @@ -480,18 +480,15 @@ void add_to_alternates_file(const char *reference) strbuf_release(&line); fclose(in); - - if (found) { - rollback_lock_file(lock); - lock = NULL; - } } else if (errno != ENOENT) die_errno("unable to read alternates file"); - if (lock) { + if (found) { + rollback_lock_file(&lock); + } else { fprintf_or_die(out, "%s\n", reference); - if (commit_lock_file(lock)) + if (commit_lock_file(&lock)) die_errno("unable to move new alternates file into place"); if (alt_odb_tail) link_alt_odb_entries(reference, '\n', NULL, 0); -- cgit v0.10.2-6-g49f6