1084b845d9
It was stupid to link the same element twice to lock_file_list and end up in a loop, so we certainly need a fix. But it is not like we are taking a lock on multiple files in this case. It is just that we leave the linked element on the list even after commit_lock_file() successfully removes the cruft. We cannot remove the list element in commit_lock_file(); if we are interrupted in the middle of list manipulation, the call to remove_lock_file_on_signal() will happen with a broken list structure pointed by lock_file_list, which would cause the cruft to remain, so not removing the list element is the right thing to do. Instead we should be reusing the element already on the list. There is already a code for that in lock_file() function in lockfile.c. The code checks lk->next and the element is linked only when it is not already on the list -- which is incorrect for the last element on the list (which has NULL in its next field), but if you read the check as "is this element already on the list?" it actually makes sense. We do not want to link it on the list again, nor we would want to set up signal/atexit over and over. Signed-off-by: Junio C Hamano <junkio@cox.net>
76 lines
1.5 KiB
C
76 lines
1.5 KiB
C
/*
|
|
* Copyright (c) 2005, Junio C Hamano
|
|
*/
|
|
#include <signal.h>
|
|
#include "cache.h"
|
|
|
|
static struct lock_file *lock_file_list;
|
|
|
|
static void remove_lock_file(void)
|
|
{
|
|
while (lock_file_list) {
|
|
if (lock_file_list->filename[0])
|
|
unlink(lock_file_list->filename);
|
|
lock_file_list = lock_file_list->next;
|
|
}
|
|
}
|
|
|
|
static void remove_lock_file_on_signal(int signo)
|
|
{
|
|
remove_lock_file();
|
|
signal(SIGINT, SIG_DFL);
|
|
raise(signo);
|
|
}
|
|
|
|
static int lock_file(struct lock_file *lk, const char *path)
|
|
{
|
|
int fd;
|
|
sprintf(lk->filename, "%s.lock", path);
|
|
fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
|
|
if (0 <= fd) {
|
|
if (!lk->on_list) {
|
|
lk->next = lock_file_list;
|
|
lock_file_list = lk;
|
|
lk->on_list = 1;
|
|
}
|
|
if (lock_file_list) {
|
|
signal(SIGINT, remove_lock_file_on_signal);
|
|
atexit(remove_lock_file);
|
|
}
|
|
if (adjust_shared_perm(lk->filename))
|
|
return error("cannot fix permission bits on %s",
|
|
lk->filename);
|
|
}
|
|
else
|
|
lk->filename[0] = 0;
|
|
return fd;
|
|
}
|
|
|
|
int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on_error)
|
|
{
|
|
int fd = lock_file(lk, path);
|
|
if (fd < 0 && die_on_error)
|
|
die("unable to create '%s': %s", path, strerror(errno));
|
|
return fd;
|
|
}
|
|
|
|
int commit_lock_file(struct lock_file *lk)
|
|
{
|
|
char result_file[PATH_MAX];
|
|
int i;
|
|
strcpy(result_file, lk->filename);
|
|
i = strlen(result_file) - 5; /* .lock */
|
|
result_file[i] = 0;
|
|
i = rename(lk->filename, result_file);
|
|
lk->filename[0] = 0;
|
|
return i;
|
|
}
|
|
|
|
void rollback_lock_file(struct lock_file *lk)
|
|
{
|
|
if (lk->filename[0])
|
|
unlink(lk->filename);
|
|
lk->filename[0] = 0;
|
|
}
|
|
|