sha1_file: do not leak `lock_file`

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 <peff@peff.net> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed Oct 5, 2017 at 22:32 UTC f132a127eebb8f1b14b87fc317ca68278fceb2e8
1 file changed +8 -11
sha1_file.c
+8 -11
@@ -456,19 +456,19 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
456
457 void add_to_alternates_file(const char *reference)
458 {
459 - struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
459 + struct lock_file lock = LOCK_INIT;
460 char *alts = git_pathdup("objects/info/alternates");
461 FILE *in, *out;
462 + int found = 0;
463
463 - hold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR);
464 - out = fdopen_lock_file(lock, "w");
464 + hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);
465 + out = fdopen_lock_file(&lock, "w");
466 if (!out)
467 die_errno("unable to fdopen alternates lockfile");
468
469 in = fopen(alts, "r");
470 if (in) {
471 struct strbuf line = STRBUF_INIT;
471 - int found = 0;
472
473 while (strbuf_getline(&line, in) != EOF) {
474 if (!strcmp(reference, line.buf)) {
@@ -480,18 +480,15 @@ void add_to_alternates_file(const char *reference)
480
481 strbuf_release(&line);
482 fclose(in);
483 -
484 - if (found) {
485 - rollback_lock_file(lock);
486 - lock = NULL;
487 - }
483 }
484 else if (errno != ENOENT)
485 die_errno("unable to read alternates file");
486
492 - if (lock) {
487 + if (found) {
488 + rollback_lock_file(&lock);
489 + } else {
490 fprintf_or_die(out, "%s\n", reference);
494 - if (commit_lock_file(lock))
491 + if (commit_lock_file(&lock))
492 die_errno("unable to move new alternates file into place");
493 if (alt_odb_tail)
494 link_alt_odb_entries(reference, '\n', NULL, 0);