From 013870cd2cb1b0d6719a7a9123e126a62426520b Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 13:14:47 +0200 Subject: 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 Signed-off-by: Junio C Hamano diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt index d4484d1..93b5f23 100644 --- 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`. -* 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: @@ -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 -`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. @@ -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. +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 @@ -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 - `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. diff --git a/lockfile.c b/lockfile.c index d27e61c..e046027 100644 --- a/lockfile.c +++ b/lockfile.c @@ -7,20 +7,29 @@ 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) { - 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); + } 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) { - remove_lock_files(); + remove_lock_files(1); 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); - atexit(remove_lock_files); + atexit(remove_lock_files_on_exit); } 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; + lk->fp = NULL; 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; } +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) @@ -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; + FILE *fp = lk->fp; + int err; 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; } + return 0; } diff --git a/lockfile.h b/lockfile.h index 9059e89..dc066d1 100644 --- 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 + * - 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()). @@ -56,6 +58,7 @@ struct lock_file { 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; @@ -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 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 *); -- cgit v0.10.2-6-g49f6