expire_reflog(): use a lock_file for rewriting the reflog file
We don't actually need the locking functionality, because we already hold the lock on the reference itself, which is how the reflog file is locked. But the lock_file code can do some of the bookkeeping for us, and it is more careful than the old code here was. For example: * It correctly handles the case that the reflog lock file already exists for some reason or cannot be opened. * It correctly cleans up the lockfile if the program dies. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Michael Haggerty committed
Dec 12, 2014 at 09:56 UTC
f3b661f766c1525265d065fdd0529985979c80c6
1 file changed
+41
-19
builtin/reflog.c
+41
-19
@@ -352,9 +352,10 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int
352
static int expire_reflog(const char *refname, const unsigned char *sha1,
353
struct cmd_reflog_expire_cb *cmd)
354
{
355
+ static struct lock_file reflog_lock;
356
struct expire_reflog_cb cb;
357
struct ref_lock *lock;
357
- char *log_file, *newlog_path = NULL;
358
+ char *log_file;
359
struct commit *tip_commit;
360
struct commit_list *tips;
361
int status = 0;
@@ -362,8 +363,9 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,
363
memset(&cb, 0, sizeof(cb));
364
365
/*
365
- * we take the lock for the ref itself to prevent it from
366
- * getting updated.
366
+ * The reflog file is locked by holding the lock on the
367
+ * reference itself, plus we might need to update the
368
+ * reference if --updateref was specified:
369
*/
370
lock = lock_any_ref_for_update(refname, sha1, 0, NULL);
371
if (!lock)
@@ -372,10 +374,29 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,
374
unlock_ref(lock);
375
return 0;
376
}
377
+
378
log_file = git_pathdup("logs/%s", refname);
379
if (!cmd->dry_run) {
377
- newlog_path = git_pathdup("logs/%s.lock", refname);
378
- cb.newlog = fopen(newlog_path, "w");
380
+ /*
381
+ * Even though holding $GIT_DIR/logs/$reflog.lock has
382
+ * no locking implications, we use the lock_file
383
+ * machinery here anyway because it does a lot of the
384
+ * work we need, including cleaning up if the program
385
+ * exits unexpectedly.
386
+ */
387
+ if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) {
388
+ struct strbuf err = STRBUF_INIT;
389
+ unable_to_lock_message(log_file, errno, &err);
390
+ error("%s", err.buf);
391
+ strbuf_release(&err);
392
+ goto failure;
393
+ }
394
+ cb.newlog = fdopen_lock_file(&reflog_lock, "w");
395
+ if (!cb.newlog) {
396
+ error("cannot fdopen %s (%s)",
397
+ reflog_lock.filename.buf, strerror(errno));
398
+ goto failure;
399
+ }
400
}
401
402
cb.cmd = cmd;
@@ -423,32 +444,33 @@ static int expire_reflog(const char *refname, const unsigned char *sha1,
444
}
445
446
if (cb.newlog) {
426
- if (fclose(cb.newlog)) {
427
- status |= error("%s: %s", strerror(errno),
428
- newlog_path);
429
- unlink(newlog_path);
447
+ if (close_lock_file(&reflog_lock)) {
448
+ status |= error("couldn't write %s: %s", log_file,
449
+ strerror(errno));
450
} else if (cmd->updateref &&
451
(write_in_full(lock->lock_fd,
452
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
453
write_str_in_full(lock->lock_fd, "\n") != 1 ||
454
close_ref(lock) < 0)) {
435
- status |= error("Couldn't write %s",
455
+ status |= error("couldn't write %s",
456
lock->lk->filename.buf);
437
- unlink(newlog_path);
438
- } else if (rename(newlog_path, log_file)) {
439
- status |= error("cannot rename %s to %s",
440
- newlog_path, log_file);
441
- unlink(newlog_path);
457
+ rollback_lock_file(&reflog_lock);
458
+ } else if (commit_lock_file(&reflog_lock)) {
459
+ status |= error("unable to commit reflog '%s' (%s)",
460
+ log_file, strerror(errno));
461
} else if (cmd->updateref && commit_ref(lock)) {
443
- status |= error("Couldn't set %s", lock->ref_name);
444
- } else {
445
- adjust_shared_perm(log_file);
462
+ status |= error("couldn't set %s", lock->ref_name);
463
}
464
}
448
- free(newlog_path);
465
free(log_file);
466
unlock_ref(lock);
467
return status;
468
+
469
+ failure:
470
+ rollback_lock_file(&reflog_lock);
471
+ free(log_file);
472
+ unlock_ref(lock);
473
+ return -1;
474
}
475
476
static int collect_reflog(const char *ref, const unsigned char *sha1, int unused, void *cb_data)