Commit 19d9beb7 by Carlos Martín Nieto

filebuf: remove lockfile upon rename errors

When we have an error renaming the lockfile, we need to make sure
that we remove it upon cleanup. For this, we need to keep track of
whether we opened the file and whether the rename succeeded.

If we did create the lockfile but the rename did not succeed, we
remove the lockfile. This won't protect against all errors, but
the most common ones (target file is open) does get handled.
parent 668053be
...@@ -101,7 +101,7 @@ void git_filebuf_cleanup(git_filebuf *file) ...@@ -101,7 +101,7 @@ void git_filebuf_cleanup(git_filebuf *file)
if (file->fd_is_open && file->fd >= 0) if (file->fd_is_open && file->fd >= 0)
p_close(file->fd); p_close(file->fd);
if (file->fd_is_open && file->path_lock && git_path_exists(file->path_lock)) if (file->created_lock && !file->did_rename && file->path_lock && git_path_exists(file->path_lock))
p_unlink(file->path_lock); p_unlink(file->path_lock);
if (file->compute_digest) { if (file->compute_digest) {
...@@ -258,6 +258,7 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode ...@@ -258,6 +258,7 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode
goto cleanup; goto cleanup;
} }
file->fd_is_open = true; file->fd_is_open = true;
file->created_lock = true;
/* No original path */ /* No original path */
file->path_original = NULL; file->path_original = NULL;
...@@ -281,6 +282,8 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode ...@@ -281,6 +282,8 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode
/* open the file for locking */ /* open the file for locking */
if ((error = lock_file(file, flags, mode)) < 0) if ((error = lock_file(file, flags, mode)) < 0)
goto cleanup; goto cleanup;
file->created_lock = true;
} }
return 0; return 0;
...@@ -340,6 +343,8 @@ int git_filebuf_commit(git_filebuf *file) ...@@ -340,6 +343,8 @@ int git_filebuf_commit(git_filebuf *file)
goto on_error; goto on_error;
} }
file->did_rename = true;
git_filebuf_cleanup(file); git_filebuf_cleanup(file);
return 0; return 0;
......
...@@ -44,6 +44,8 @@ struct git_filebuf { ...@@ -44,6 +44,8 @@ struct git_filebuf {
size_t buf_size, buf_pos; size_t buf_size, buf_pos;
git_file fd; git_file fd;
bool fd_is_open; bool fd_is_open;
bool created_lock;
bool did_rename;
bool do_not_buffer; bool do_not_buffer;
int last_error; int last_error;
}; };
......
...@@ -148,6 +148,6 @@ void test_core_filebuf__rename_error(void) ...@@ -148,6 +148,6 @@ void test_core_filebuf__rename_error(void)
p_close(fd); p_close(fd);
git_filebuf_cleanup(&file); git_filebuf_cleanup(&file);
cl_assert_equal_i(false, git_path_exists(test_lock));
cl_assert_equal_i(false, git_path_exists(test_lock));
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment