commit_lock_file_to(): refactor a helper out of commit_lock_file()
commit_locked_index(), when writing to an alternate index file, duplicates (poorly) the code in commit_lock_file(). And anyway, it shouldn't have to know so much about the internal workings of lockfile objects. So extract a new function commit_lock_file_to() that does the work common to the two functions, and call it from both commit_lock_file() and commit_locked_index(). 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 12:28 UTC
751bacedaa507b7b6d10b2c1f48e019a01a8fa6e
4 files changed
+50
-38
Documentation/technical/api-lockfile.txt
+20
-14
@@ -49,14 +49,14 @@ The caller:
49
When finished writing, the caller can:
50
51
* Close the file descriptor and rename the lockfile to its final
52
- destination by calling `commit_lock_file`.
52
+ destination by calling `commit_lock_file` or `commit_lock_file_to`.
53
54
* Close the file descriptor and remove the lockfile by calling
55
`rollback_lock_file`.
56
57
* Close the file descriptor without removing or renaming the lockfile
58
by calling `close_lock_file`, and later call `commit_lock_file`,
59
- `rollback_lock_file`, or `reopen_lock_file`.
59
+ `commit_lock_file_to`, `rollback_lock_file`, or `reopen_lock_file`.
60
61
Even after the lockfile is committed or rolled back, the `lock_file`
62
object must not be freed or altered by the caller. However, it may be
@@ -64,20 +64,19 @@ reused; just pass it to another call of `hold_lock_file_for_update` or
64
`hold_lock_file_for_append`.
65
66
If the program exits before you have called one of `commit_lock_file`,
67
-`rollback_lock_file`, or `close_lock_file`, an `atexit(3)` handler
68
-will close and remove the lockfile, rolling back any uncommitted
69
-changes.
67
+`commit_lock_file_to`, `rollback_lock_file`, or `close_lock_file`, an
68
+`atexit(3)` handler will close and remove the lockfile, rolling back
69
+any uncommitted changes.
70
71
If you need to close the file descriptor you obtained from a
72
`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 later call to
76
-`commit_lock_file` or `rollback_lock_file` or program exit would
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
and then later open another file descriptor for a completely different
79
-purpose, then a call to `commit_lock_file` or `rollback_lock_file`
80
-might close that unrelated file descriptor.
78
+purpose, then a commit or rollback might close that unrelated file
79
+descriptor.
80
81
82
Error handling
@@ -100,9 +99,9 @@ unable_to_lock_die::
99
100
Emit an appropriate error message and `die()`.
101
103
-Similarly, `commit_lock_file` and `close_lock_file` return 0 on
104
-success. On failure they set `errno` appropriately, do their best to
105
-roll back the lockfile, and return -1.
102
+Similarly, `commit_lock_file`, `commit_lock_file_to`, and
103
+`close_lock_file` return 0 on success. On failure they set `errno`
104
+appropriately, do their best to roll back the lockfile, and return -1.
105
106
107
Flags
@@ -156,6 +155,12 @@ commit_lock_file::
155
`commit_lock_file` for a `lock_file` object that is not
156
currently locked.
157
158
+commit_lock_file_to::
159
+
160
+ Like `commit_lock_file()`, except that it takes an explicit
161
+ `path` argument to which the lockfile should be renamed. The
162
+ `path` must be on the same filesystem as the lock file.
163
+
164
rollback_lock_file::
165
166
Take a pointer to the `struct lock_file` initialized with an
@@ -172,8 +177,9 @@ close_lock_file::
177
`hold_lock_file_for_append`, and close the file descriptor.
178
Return 0 upon success. On failure to `close(2)`, return a
179
negative value and roll back the lock file. Usually
175
- `commit_lock_file` or `rollback_lock_file` should eventually
176
- be called if `close_lock_file` succeeds.
180
+ `commit_lock_file`, `commit_lock_file_to`, or
181
+ `rollback_lock_file` should eventually be called if
182
+ `close_lock_file` succeeds.
183
184
reopen_lock_file::
185
cache.h
+1
@@ -590,6 +590,7 @@ extern void unable_to_lock_message(const char *path, int err,
590
extern NORETURN void unable_to_lock_die(const char *path, int err);
591
extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
592
extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
593
+extern int commit_lock_file_to(struct lock_file *, const char *path);
594
extern int commit_lock_file(struct lock_file *);
595
extern int reopen_lock_file(struct lock_file *);
596
extern void update_index_if_able(struct index_state *, struct lock_file *);
lockfile.c
+26
-14
@@ -43,9 +43,9 @@
43
* Same as the previous state, except that the lockfile is closed
44
* and fd is -1.
45
*
46
- * - Unlocked (after commit_lock_file(), rollback_lock_file(), a
47
- * failed attempt to lock, or a failed close_lock_file()). In this
48
- * state:
46
+ * - Unlocked (after commit_lock_file(), commit_lock_file_to(),
47
+ * rollback_lock_file(), a failed attempt to lock, or a failed
48
+ * close_lock_file()). In this state:
49
* - active is unset
50
* - filename is empty (usually, though there are transitory
51
* states in which this condition doesn't hold). Client code should
@@ -284,23 +284,15 @@ int reopen_lock_file(struct lock_file *lk)
284
return lk->fd;
285
}
286
287
-int commit_lock_file(struct lock_file *lk)
287
+int commit_lock_file_to(struct lock_file *lk, const char *path)
288
{
289
- static struct strbuf result_file = STRBUF_INIT;
290
- int err;
291
-
289
if (!lk->active)
293
- die("BUG: attempt to commit unlocked object");
290
+ die("BUG: attempt to commit unlocked object to \"%s\"", path);
291
292
if (close_lock_file(lk))
293
return -1;
294
298
- /* remove ".lock": */
299
- strbuf_add(&result_file, lk->filename.buf,
300
- lk->filename.len - LOCK_SUFFIX_LEN);
301
- err = rename(lk->filename.buf, result_file.buf);
302
- strbuf_reset(&result_file);
303
- if (err) {
295
+ if (rename(lk->filename.buf, path)) {
296
int save_errno = errno;
297
rollback_lock_file(lk);
298
errno = save_errno;
@@ -312,6 +304,26 @@ int commit_lock_file(struct lock_file *lk)
304
return 0;
305
}
306
307
+int commit_lock_file(struct lock_file *lk)
308
+{
309
+ static struct strbuf result_file = STRBUF_INIT;
310
+ int err;
311
+
312
+ if (!lk->active)
313
+ die("BUG: attempt to commit unlocked object");
314
+
315
+ if (lk->filename.len <= LOCK_SUFFIX_LEN ||
316
+ strcmp(lk->filename.buf + lk->filename.len - LOCK_SUFFIX_LEN, LOCK_SUFFIX))
317
+ die("BUG: lockfile filename corrupt");
318
+
319
+ /* remove ".lock": */
320
+ strbuf_add(&result_file, lk->filename.buf,
321
+ lk->filename.len - LOCK_SUFFIX_LEN);
322
+ err = commit_lock_file_to(lk, result_file.buf);
323
+ strbuf_reset(&result_file);
324
+ return err;
325
+}
326
+
327
int hold_locked_index(struct lock_file *lk, int die_on_error)
328
{
329
return hold_lock_file_for_update(lk, get_index_file(),
read-cache.c
+3
-10
@@ -2041,17 +2041,10 @@ void set_alternate_index_output(const char *name)
2041
2042
static int commit_locked_index(struct lock_file *lk)
2043
{
2044
- if (alternate_index_output) {
2045
- if (close_lock_file(lk))
2046
- return -1;
2047
- if (rename(lk->filename.buf, alternate_index_output))
2048
- return -1;
2049
- lk->active = 0;
2050
- strbuf_reset(&lk->filename);
2051
- return 0;
2052
- } else {
2044
+ if (alternate_index_output)
2045
+ return commit_lock_file_to(lk, alternate_index_output);
2046
+ else
2047
return commit_lock_file(lk);
2054
- }
2048
}
2049
2050
static int do_write_locked_index(struct index_state *istate, struct lock_file *lock,