fdopen_lock_file(): access a lockfile using stdio

Add a new function, fdopen_lock_file(), which returns a FILE pointer
open to the lockfile. If a stream is open on a lock_file object, it is
closed using fclose() on commit, rollback, or close_lock_file().

This change will allow callers to use stdio to write to a lockfile
without having to muck around in the internal representation of the
lock_file object (callers will be rewritten in upcoming commits).

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt
index d4484d154d553b55f3f92811e9737c8d7079cf19..93b5f23e4c8a1bb45dc26d5f9cec402df7f57ee4 100644 (file)
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -42,9 +42,13 @@ The caller:
   of the final destination (e.g. `$GIT_DIR/index`) to
   `hold_lock_file_for_update` or `hold_lock_file_for_append`.
 
   of the final destination (e.g. `$GIT_DIR/index`) to
   `hold_lock_file_for_update` or `hold_lock_file_for_append`.
 

-* Writes new content for the destination file by writing to the file
-  descriptor returned by those functions (also available via
-  `lock->fd`).
+* Writes new content for the destination file by either:
+
+  * writing to the file descriptor returned by the `hold_lock_file_*`
+    functions (also available via `lock->fd`).
+
+  * calling `fdopen_lock_file` to get a `FILE` pointer for the open
+    file and writing to the file using stdio.

 
 When finished writing, the caller can:
 
 
 When finished writing, the caller can:
 


@@ -70,10 +74,10 @@ any uncommitted changes.
 
 If you need to close the file descriptor you obtained from a
 `hold_lock_file_*` function yourself, do so by calling
 
 If you need to close the file descriptor you obtained from a
 `hold_lock_file_*` function yourself, do so by calling

-`close_lock_file`. You should never call `close(2)` yourself!
-Otherwise the `struct lock_file` structure would still think that the
-file descriptor needs to be closed, and a commit or rollback would
-result in duplicate calls to `close(2)`. Worse yet, if you `close(2)`
+`close_lock_file`. You should never call `close(2)` or `fclose(3)`
+yourself! Otherwise the `struct lock_file` structure would still think
+that the file descriptor needs to be closed, and a commit or rollback
+would result in duplicate calls to `close(2)`. Worse yet, if you close

 and then later open another file descriptor for a completely different
 purpose, then a commit or rollback might close that unrelated file
 descriptor.
 and then later open another file descriptor for a completely different
 purpose, then a commit or rollback might close that unrelated file
 descriptor.


@@ -143,6 +147,13 @@ hold_lock_file_for_append::
        the existing contents of the file (if any) to the lockfile and
        position its write pointer at the end of the file.
 
        the existing contents of the file (if any) to the lockfile and
        position its write pointer at the end of the file.
 

+fdopen_lock_file::
+
+       Associate a stdio stream with the lockfile. Return NULL
+       (*without* rolling back the lockfile) on error. The stream is
+       closed automatically when `close_lock_file` is called or when
+       the file is committed or rolled back.
+

 get_locked_file_path::
 
        Return the path of the file that is locked by the specified
 get_locked_file_path::
 
        Return the path of the file that is locked by the specified


@@ -179,10 +190,11 @@ close_lock_file::
 
        Take a pointer to the `struct lock_file` initialized with an
        earlier call to `hold_lock_file_for_update` or
 
        Take a pointer to the `struct lock_file` initialized with an
        earlier call to `hold_lock_file_for_update` or

-       `hold_lock_file_for_append`, and close the file descriptor.
-       Return 0 upon success. On failure to `close(2)`, return a
-       negative value and roll back the lock file. Usually
-       `commit_lock_file`, `commit_lock_file_to`, or
+       `hold_lock_file_for_append`. Close the file descriptor (and
+       the file pointer if it has been opened using
+       `fdopen_lock_file`). Return 0 upon success. On failure to
+       `close(2)`, return a negative value and roll back the lock
+       file. Usually `commit_lock_file`, `commit_lock_file_to`, or

        `rollback_lock_file` should eventually be called if
        `close_lock_file` succeeds.
 
        `rollback_lock_file` should eventually be called if
        `close_lock_file` succeeds.
 

diff --git a/lockfile.c b/lockfile.c
index d27e61cafcfd2a8277e67edddaf41c29065babee..e0460275e1904e12728840102d0a51e98008eef9 100644 (file)
--- a/lockfile.c
+++ b/lockfile.c
@@ -7,20 +7,29 @@
 
 static struct lock_file *volatile lock_file_list;
 
 
 static struct lock_file *volatile lock_file_list;
 

-static void remove_lock_files(void)
+static void remove_lock_files(int skip_fclose)

 {
        pid_t me = getpid();
 
        while (lock_file_list) {
 {
        pid_t me = getpid();
 
        while (lock_file_list) {

-               if (lock_file_list->owner == me)
+               if (lock_file_list->owner == me) {
+                       /* fclose() is not safe to call in a signal handler */
+                       if (skip_fclose)
+                               lock_file_list->fp = NULL;

                        rollback_lock_file(lock_file_list);
                        rollback_lock_file(lock_file_list);

+               }

                lock_file_list = lock_file_list->next;
        }
 }
 
                lock_file_list = lock_file_list->next;
        }
 }
 

+static void remove_lock_files_on_exit(void)
+{
+       remove_lock_files(0);
+}
+

 static void remove_lock_files_on_signal(int signo)
 {
 static void remove_lock_files_on_signal(int signo)
 {

-       remove_lock_files();
+       remove_lock_files(1);

        sigchain_pop(signo);
        raise(signo);
 }
        sigchain_pop(signo);
        raise(signo);
 }


@@ -97,7 +106,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
        if (!lock_file_list) {
                /* One-time initialization */
                sigchain_push_common(remove_lock_files_on_signal);
        if (!lock_file_list) {
                /* One-time initialization */
                sigchain_push_common(remove_lock_files_on_signal);

-               atexit(remove_lock_files);
+               atexit(remove_lock_files_on_exit);

        }
 
        if (lk->active)
        }
 
        if (lk->active)


@@ -106,6 +115,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
        if (!lk->on_list) {
                /* Initialize *lk and add it to lock_file_list: */
                lk->fd = -1;
        if (!lk->on_list) {
                /* Initialize *lk and add it to lock_file_list: */
                lk->fd = -1;

+               lk->fp = NULL;

                lk->active = 0;
                lk->owner = 0;
                strbuf_init(&lk->filename, pathlen + LOCK_SUFFIX_LEN);
                lk->active = 0;
                lk->owner = 0;
                strbuf_init(&lk->filename, pathlen + LOCK_SUFFIX_LEN);


@@ -214,6 +224,17 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
        return fd;
 }
 
        return fd;
 }
 

+FILE *fdopen_lock_file(struct lock_file *lk, const char *mode)
+{
+       if (!lk->active)
+               die("BUG: fdopen_lock_file() called for unlocked object");
+       if (lk->fp)
+               die("BUG: fdopen_lock_file() called twice for file '%s'", lk->filename.buf);
+
+       lk->fp = fdopen(lk->fd, mode);
+       return lk->fp;
+}
+

 char *get_locked_file_path(struct lock_file *lk)
 {
        if (!lk->active)
 char *get_locked_file_path(struct lock_file *lk)
 {
        if (!lk->active)


@@ -226,17 +247,32 @@ char *get_locked_file_path(struct lock_file *lk)
 int close_lock_file(struct lock_file *lk)
 {
        int fd = lk->fd;
 int close_lock_file(struct lock_file *lk)
 {
        int fd = lk->fd;

+       FILE *fp = lk->fp;
+       int err;

 
        if (fd < 0)
                return 0;
 
        lk->fd = -1;
 
        if (fd < 0)
                return 0;
 
        lk->fd = -1;

-       if (close(fd)) {
+       if (fp) {
+               lk->fp = NULL;
+
+               /*
+                * Note: no short-circuiting here; we want to fclose()
+                * in any case!
+                */
+               err = ferror(fp) | fclose(fp);
+       } else {
+               err = close(fd);
+       }
+
+       if (err) {

                int save_errno = errno;
                rollback_lock_file(lk);
                errno = save_errno;
                return -1;
        }
                int save_errno = errno;
                rollback_lock_file(lk);
                errno = save_errno;
                return -1;
        }

+

        return 0;
 }
 
        return 0;
 }
 

diff --git a/lockfile.h b/lockfile.h
index 9059e8958f555357b189be528380cec536ff6dee..dc066d1783327844f256892f75ab097d7ea8493d 100644 (file)
--- a/lockfile.h
+++ b/lockfile.h
@@ -34,6 +34,8 @@
  *   - active is set
  *   - filename holds the filename of the lockfile
  *   - fd holds a file descriptor open for writing to the lockfile
  *   - active is set
  *   - filename holds the filename of the lockfile
  *   - fd holds a file descriptor open for writing to the lockfile

+ *   - fp holds a pointer to an open FILE object if and only if
+ *     fdopen_lock_file() has been called on the object

  *   - owner holds the PID of the process that locked the file
  *
  * - Locked, lockfile closed (after successful close_lock_file()).
  *   - owner holds the PID of the process that locked the file
  *
  * - Locked, lockfile closed (after successful close_lock_file()).


@@ -56,6 +58,7 @@ struct lock_file {
        struct lock_file *volatile next;
        volatile sig_atomic_t active;
        volatile int fd;
        struct lock_file *volatile next;
        volatile sig_atomic_t active;
        volatile int fd;

+       FILE *volatile fp;

        volatile pid_t owner;
        char on_list;
        struct strbuf filename;
        volatile pid_t owner;
        char on_list;
        struct strbuf filename;


@@ -74,6 +77,7 @@ extern void unable_to_lock_message(const char *path, int err,
 extern NORETURN void unable_to_lock_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
 extern NORETURN void unable_to_lock_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);

+extern FILE *fdopen_lock_file(struct lock_file *, const char *mode);

 extern char *get_locked_file_path(struct lock_file *);
 extern int commit_lock_file_to(struct lock_file *, const char *path);
 extern int commit_lock_file(struct lock_file *);
 extern char *get_locked_file_path(struct lock_file *);
 extern int commit_lock_file_to(struct lock_file *, const char *path);
 extern int commit_lock_file(struct lock_file *);