close_lock_file(): new function in the lockfile API

The lockfile API is a handy way to obtain a file that is cleaned
up if you die().  But sometimes you would need this sequence to
work:

 1. hold_lock_file_for_update() to get a file descriptor for
    writing;

 2. write the contents out, without being able to decide if the
    results should be committed or rolled back;

 3. do something else that makes the decision --- and this
    "something else" needs the lockfile not to have an open file
    descriptor for writing (e.g. Windows do not want a open file
    to be renamed);

 4. call commit_lock_file() or rollback_lock_file() as
    appropriately.

This adds close_lock_file() you can call between step 2 and 3 in
the above sequence.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt
index 5b1553e52c83a1e8e8a913b8303a0bfab10ec377..dd894043ae8b04269b3aa2108f96cb935217181d 100644 (file)
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -37,7 +37,8 @@ commit_lock_file::
        Take a pointer to the `struct lock_file` initialized
        with an earlier call to `hold_lock_file_for_update()`,
        close the file descriptor and rename the lockfile to its
-       final destination.
+       final destination.  Returns 0 upon success, a negative
+       value on failure to close(2) or rename(2).
 
 rollback_lock_file::
 
@@ -45,6 +46,12 @@ rollback_lock_file::
        with an earlier call to `hold_lock_file_for_update()`,
        close the file descriptor and remove the lockfile.
 
+close_lock_file::
+       Take a pointer to the `struct lock_file` initialized
+       with an earlier call to `hold_lock_file_for_update()`,
+       and close the file descriptor.  Returns 0 upon success,
+       a negative value on failure to close(2).
+
 Because the structure is used in an `atexit(3)` handler, its
 storage has to stay throughout the life of the program.  It
 cannot be an auto variable allocated on the stack.
@@ -54,8 +61,10 @@ done writing to the file descriptor.  If you do not call either
 and simply `exit(3)` from the program, an `atexit(3)` handler
 will close and remove the lockfile.
 
-You should not close the file descriptor you obtained from
-`hold_lock_file_for_update` function yourself.  The `struct
+If you need to close the file descriptor you obtained from
+`hold_lock_file_for_update` function yourself, do so by calling
+`close_lock_file()`.  You should never call `close(2)` yourself!
+Otherwise the `struct
 lock_file` structure still remembers that the file descriptor
 needs to be closed, and a later call to `commit_lock_file()` or
 `rollback_lock_file()` will result in duplicate calls to

diff --git a/cache.h b/cache.h
index 39331c28be15605f4b943cea22a7a8caa5b501c2..24735bdfeeca1ca1b8fc9579ce029d1faeb978e3 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -308,7 +308,7 @@ extern int commit_lock_file(struct lock_file *);
 extern int hold_locked_index(struct lock_file *, int);
 extern int commit_locked_index(struct lock_file *);
 extern void set_alternate_index_output(const char *);
-
+extern int close_lock_file(struct lock_file *);
 extern void rollback_lock_file(struct lock_file *);
 extern int delete_ref(const char *, const unsigned char *sha1);
 

diff --git a/lockfile.c b/lockfile.c
index f45d3ed54454635b84cfe9b8c1e7d99d33ffd4d9..663f18f9c40145e5ab772021e0c700e9123a4b01 100644 (file)
--- a/lockfile.c
+++ b/lockfile.c
@@ -13,7 +13,8 @@ static void remove_lock_file(void)
        while (lock_file_list) {
                if (lock_file_list->owner == me &&
                    lock_file_list->filename[0]) {
-                       close(lock_file_list->fd);
+                       if (lock_file_list->fd >= 0)
+                               close(lock_file_list->fd);
                        unlink(lock_file_list->filename);
                }
                lock_file_list = lock_file_list->next;
@@ -159,17 +160,26 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on
        return fd;
 }
 
+int close_lock_file(struct lock_file *lk)
+{
+       int fd = lk->fd;
+       lk->fd = -1;
+       return close(fd);
+}
+
 int commit_lock_file(struct lock_file *lk)
 {
        char result_file[PATH_MAX];
-       int i;
-       close(lk->fd);
+       size_t i;
+       if (lk->fd >= 0 && close_lock_file(lk))
+               return -1;
        strcpy(result_file, lk->filename);
        i = strlen(result_file) - 5; /* .lock */
        result_file[i] = 0;
-       i = rename(lk->filename, result_file);
+       if (rename(lk->filename, result_file))
+               return -1;
        lk->filename[0] = 0;
-       return i;
+       return 0;
 }
 
 int hold_locked_index(struct lock_file *lk, int die_on_error)
@@ -185,9 +195,12 @@ void set_alternate_index_output(const char *name)
 int commit_locked_index(struct lock_file *lk)
 {
        if (alternate_index_output) {
-               int result = rename(lk->filename, alternate_index_output);
+               if (lk->fd >= 0 && close_lock_file(lk))
+                       return -1;
+               if (rename(lk->filename, alternate_index_output))
+                       return -1;
                lk->filename[0] = 0;
-               return result;
+               return 0;
        }
        else
                return commit_lock_file(lk);
@@ -196,7 +209,8 @@ int commit_locked_index(struct lock_file *lk)
 void rollback_lock_file(struct lock_file *lk)
 {
        if (lk->filename[0]) {
-               close(lk->fd);
+               if (lk->fd >= 0)
+                       close(lk->fd);
                unlink(lk->filename);
        }
        lk->filename[0] = 0;