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>

Michael Haggerty committed Oct 1, 2014 at 13:14 UTC 013870cd2cb1b0d6719a7a9123e126a62426520b
3 files changed +68 -16
Documentation/technical/api-lockfile.txt
+23 -11
@@ -42,9 +42,13 @@ The caller:
42 of the final destination (e.g. `$GIT_DIR/index`) to
43 `hold_lock_file_for_update` or `hold_lock_file_for_append`.
44
45 -* Writes new content for the destination file by writing to the file
46 - descriptor returned by those functions (also available via
47 - `lock->fd`).
45 +* Writes new content for the destination file by either:
46 +
47 + * writing to the file descriptor returned by the `hold_lock_file_*`
48 + functions (also available via `lock->fd`).
49 +
50 + * calling `fdopen_lock_file` to get a `FILE` pointer for the open
51 + file and writing to the file using stdio.
52
53 When finished writing, the caller can:
54
@@ -70,10 +74,10 @@ any uncommitted changes.
74
75 If you need to close the file descriptor you obtained from a
76 `hold_lock_file_*` function yourself, do so by calling
73 -`close_lock_file`. You should never call `close(2)` yourself!
74 -Otherwise the `struct lock_file` structure would still think that the
75 -file descriptor needs to be closed, and a commit or rollback would
76 -result in duplicate calls to `close(2)`. Worse yet, if you `close(2)`
77 +`close_lock_file`. You should never call `close(2)` or `fclose(3)`
78 +yourself! Otherwise the `struct lock_file` structure would still think
79 +that the file descriptor needs to be closed, and a commit or rollback
80 +would result in duplicate calls to `close(2)`. Worse yet, if you close
81 and then later open another file descriptor for a completely different
82 purpose, then a commit or rollback might close that unrelated file
83 descriptor.
@@ -143,6 +147,13 @@ hold_lock_file_for_append::
147 the existing contents of the file (if any) to the lockfile and
148 position its write pointer at the end of the file.
149
150 +fdopen_lock_file::
151 +
152 + Associate a stdio stream with the lockfile. Return NULL
153 + (*without* rolling back the lockfile) on error. The stream is
154 + closed automatically when `close_lock_file` is called or when
155 + the file is committed or rolled back.
156 +
157 get_locked_file_path::
158
159 Return the path of the file that is locked by the specified
@@ -179,10 +190,11 @@ close_lock_file::
190
191 Take a pointer to the `struct lock_file` initialized with an
192 earlier call to `hold_lock_file_for_update` or
182 - `hold_lock_file_for_append`, and close the file descriptor.
183 - Return 0 upon success. On failure to `close(2)`, return a
184 - negative value and roll back the lock file. Usually
185 - `commit_lock_file`, `commit_lock_file_to`, or
193 + `hold_lock_file_for_append`. Close the file descriptor (and
194 + the file pointer if it has been opened using
195 + `fdopen_lock_file`). Return 0 upon success. On failure to
196 + `close(2)`, return a negative value and roll back the lock
197 + file. Usually `commit_lock_file`, `commit_lock_file_to`, or
198 `rollback_lock_file` should eventually be called if
199 `close_lock_file` succeeds.
200
lockfile.c
+41 -5
@@ -7,20 +7,29 @@
7
8 static struct lock_file *volatile lock_file_list;
9
10 -static void remove_lock_files(void)
10 +static void remove_lock_files(int skip_fclose)
11 {
12 pid_t me = getpid();
13
14 while (lock_file_list) {
15 - if (lock_file_list->owner == me)
15 + if (lock_file_list->owner == me) {
16 + /* fclose() is not safe to call in a signal handler */
17 + if (skip_fclose)
18 + lock_file_list->fp = NULL;
19 rollback_lock_file(lock_file_list);
20 + }
21 lock_file_list = lock_file_list->next;
22 }
23 }
24
25 +static void remove_lock_files_on_exit(void)
26 +{
27 + remove_lock_files(0);
28 +}
29 +
30 static void remove_lock_files_on_signal(int signo)
31 {
23 - remove_lock_files();
32 + remove_lock_files(1);
33 sigchain_pop(signo);
34 raise(signo);
35 }
@@ -97,7 +106,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
106 if (!lock_file_list) {
107 /* One-time initialization */
108 sigchain_push_common(remove_lock_files_on_signal);
100 - atexit(remove_lock_files);
109 + atexit(remove_lock_files_on_exit);
110 }
111
112 if (lk->active)
@@ -106,6 +115,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
115 if (!lk->on_list) {
116 /* Initialize *lk and add it to lock_file_list: */
117 lk->fd = -1;
118 + lk->fp = NULL;
119 lk->active = 0;
120 lk->owner = 0;
121 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)
224 return fd;
225 }
226
227 +FILE *fdopen_lock_file(struct lock_file *lk, const char *mode)
228 +{
229 + if (!lk->active)
230 + die("BUG: fdopen_lock_file() called for unlocked object");
231 + if (lk->fp)
232 + die("BUG: fdopen_lock_file() called twice for file '%s'", lk->filename.buf);
233 +
234 + lk->fp = fdopen(lk->fd, mode);
235 + return lk->fp;
236 +}
237 +
238 char *get_locked_file_path(struct lock_file *lk)
239 {
240 if (!lk->active)
@@ -226,17 +247,32 @@ char *get_locked_file_path(struct lock_file *lk)
247 int close_lock_file(struct lock_file *lk)
248 {
249 int fd = lk->fd;
250 + FILE *fp = lk->fp;
251 + int err;
252
253 if (fd < 0)
254 return 0;
255
256 lk->fd = -1;
234 - if (close(fd)) {
257 + if (fp) {
258 + lk->fp = NULL;
259 +
260 + /*
261 + * Note: no short-circuiting here; we want to fclose()
262 + * in any case!
263 + */
264 + err = ferror(fp) | fclose(fp);
265 + } else {
266 + err = close(fd);
267 + }
268 +
269 + if (err) {
270 int save_errno = errno;
271 rollback_lock_file(lk);
272 errno = save_errno;
273 return -1;
274 }
275 +
276 return 0;
277 }
278
lockfile.h
+4
@@ -34,6 +34,8 @@
34 * - active is set
35 * - filename holds the filename of the lockfile
36 * - fd holds a file descriptor open for writing to the lockfile
37 + * - fp holds a pointer to an open FILE object if and only if
38 + * fdopen_lock_file() has been called on the object
39 * - owner holds the PID of the process that locked the file
40 *
41 * - Locked, lockfile closed (after successful close_lock_file()).
@@ -56,6 +58,7 @@ struct lock_file {
58 struct lock_file *volatile next;
59 volatile sig_atomic_t active;
60 volatile int fd;
61 + FILE *volatile fp;
62 volatile pid_t owner;
63 char on_list;
64 struct strbuf filename;
@@ -74,6 +77,7 @@ extern void unable_to_lock_message(const char *path, int err,
77 extern NORETURN void unable_to_lock_die(const char *path, int err);
78 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
79 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
80 +extern FILE *fdopen_lock_file(struct lock_file *, const char *mode);
81 extern char *get_locked_file_path(struct lock_file *);
82 extern int commit_lock_file_to(struct lock_file *, const char *path);
83 extern int commit_lock_file(struct lock_file *);